Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add ESM support #1142

Merged
merged 2 commits into from
Oct 7, 2024
Merged

feat: Add ESM support #1142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6be8b03
Add an ESM submodule export to the build output
goulinkh Sep 9, 2024
91c3095
Merge branch 'main' into add-esm-support
goulinkh Oct 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 0 additions & 24 deletions .babelrc
View file
Open in desktop

This file was deleted.

3 changes: 2 additions & 1 deletion .storybook/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ const config = {
"@storybook/addon-docs",
"@storybook/addon-controls",
"@storybook/addon-a11y",
"@storybook/addon-webpack5-compiler-babel"
"@storybook/addon-webpack5-compiler-babel",
],
webpackFinal: async (config) => {
process.env.BABEL_ENV = "cjs";
config.module.rules.push({
test: /\.scss$/,
use: ["style-loader", "css-loader", "sass-loader"],
Expand Down
65 changes: 65 additions & 0 deletions babel.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
const commonJs = {
presets: [
"@babel/preset-env",
"@babel/preset-react",
"@babel/preset-typescript",
],
plugins: [
"@babel/plugin-proposal-class-properties",
"babel-plugin-typescript-to-proptypes",
[
"module-resolver",
{
root: ["./src"],
alias: {
enums: "./src/enums",
components: "./src/components",
hooks: "./src/hooks",
types: "./src/types",
utils: "./src/utils",
},
},
],
],
};
const esm = {
presets: [
[
"@babel/preset-env",
{
modules: false,
targets: {
esmodules: true,
chrome: "120",
},
},
],
"@babel/preset-react",
"@babel/preset-typescript",
],
plugins: [
"@babel/plugin-proposal-class-properties",
"babel-plugin-typescript-to-proptypes",
[
"module-resolver",
{
root: ["./src"],
alias: {
enums: "./src/enums",
components: "./src/components",
hooks: "./src/hooks",
types: "./src/types",
utils: "./src/utils",
},
},
],
],
};

module.exports = {
env: {
cjs: commonJs,
test: commonJs,
esm,
},
};
12 changes: 6 additions & 6 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
"dist/**/*.js",
"dist/**/*.d.*",
"dist/**/*.scss",
"dist-types/**/*.d.ts",
"dist-esm/**/*.js",
"!dist/**/*.test.js",
"!dist/testing",
"!dist/setupTests.js"
Expand Down Expand Up @@ -128,10 +126,12 @@
"vanilla-framework": "^3.15.1 || ^4.0.0"
},
"scripts": {
"build": "rm -rf dist && yarn build-local; yarn build-declaration",
"build-local": "NODE_ENV=production babel src --out-dir dist --copy-files --no-copy-ignored --extensions '.js,.jsx,.ts,.tsx,.snap' --ignore '**/*.test.ts','**/*.test.tsx','**/*.stories.tsx','**/__snapshots__','src/setupTests.js','src/testing'",
"build-declaration": "tsc --project tsconfig.build.json && tsc-alias -p tsconfig.build.json",
"build-watch": "yarn run build-local --watch",
"build": "rm -rf dist && yarn build-cjs && yarn build-declaration && yarn build-esm && yarn build-declaration-esm",
"build-cjs": "NODE_ENV=production BABEL_ENV=cjs babel src --out-dir dist --copy-files --extensions '.js,.jsx,.ts,.tsx'",
"build-declaration": "tsc --project tsconfig.json && tsc-alias -p tsconfig.json",
"build-esm": "NODE_ENV=production BABEL_ENV=esm babel src --out-dir dist/esm --copy-files --extensions '.js,.jsx,.ts,.tsx'",
"build-declaration-esm": "tsc --project tsconfig.json --outDir dist/esm && tsc-alias -p tsconfig.json --outDir dist/esm",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe there is the need for only one declaration - this mirrors the "types" key on the package.json that I believe is missing as well

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having generated types only in the root folder /dist doesn't work for imports from /dist/esm even if the package.json has this set: "types": "dist/types/index.d.ts" because in the case of ESM import the import path prefix is not / (index) but /dist/esm which doesn't match any types path.

I found that the simplest approach would be to create a copy of the types in dist/esm to have the default TS resolution work for ESM modules.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Shouldn't we have any types key tough ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it is required since the generated type path for CJS is /dist/index.d.ts which matches the imported path @canonical/react-components (index.js). Also for ESM, the same logic applies if we use a copy of the types in /dist/esm.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can be explicit about it, but it is not required since it is automatically resolved based on the file name.

"build-watch": "yarn run build-cjs --watch",
"build-docs": "storybook build -c .storybook -o docs",
"clean": "rm -rf node_modules dist .out",
"docs": "storybook dev -p ${PORT:-9009} --no-open",
Expand Down