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.

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

WIP: rework build system to use esbuild. #1043

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

diasbruno
Copy link
Collaborator

@diasbruno diasbruno commented Feb 27, 2024

Fixes #1036.

This is an update to the build system to use esbuild.

It need to:

  • Build a pre-compiled version
  • Build a minified version
  • Use src/index as main module file
  • Run dev/examples server
  • Run tests

let
inherit (nixpkgs) pkgs;
in pkgs.mkShell {
nativeBuildInputs = with pkgs; [jq nodejs_20 swc nodePackages.typescript-language-server];
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggested change
nativeBuildInputs = with pkgs; [jq nodejs_20 swc nodePackages.typescript-language-server];
nativeBuildInputs = with pkgs; [jq nodejs_20 nodePackages.typescript-language-server];

@diasbruno
Copy link
Collaborator Author

@doeg What do you think about removing "Build an esm/cjs modules"?

I'm thinking in reducing the amount of artifacts and just a provide a minified version of the library, and, point to main file to "src/index.[tj]s" (this might fix the issue of using fork of this project.

@diasbruno
Copy link
Collaborator Author

We used to pre-compile the library because of some webpack loaders and babel transformers, but I think that if we move to functions and hooks, we can remove build step.

@doeg
Copy link

doeg commented Feb 27, 2024

@diasbruno to tell you the truth, I haven't maintained a React library before and can't give you an informed answer from that perspective. 🤔

Please correct me if I'm wrong: my understanding is that CommonJS modules are typically intended for backend/node imports. I looked at Next.js as an example of server-side imports and it looks like even they support ESM these days. I agree that CommonJS is probably not the best fit for a front-end library like this one.

A question, though! 🙇 If the module distributes just the minified version of the library, wouldn't the entrypoint file still need to use ESM module syntax (or equivalent)?

Comment on lines +26 to +33
"@typescript-eslint/eslint-plugin": "^6.21.0",
"@typescript-eslint/parser": "^6.21.0",
"@webcomponents/custom-elements": "^1.5.0",
"babel-cli": "^6.26.0",
"babel-core": "^6.25.0",
"babel-eslint": "^8.0.1",
"babel-loader": "^7.1.2",
"babel-plugin-add-module-exports": "^0.2.1",
"babel-preset-env": "^1.6.0",
"babel-preset-react": "^6.24.1",
"babel-preset-stage-2": "^6.24.1",
"coveralls": "^3.1.0",
"cross-env": "^5.2.1",
"eslint": "^4.8.0",
"eslint-config-prettier": "^2.6.0",
"eslint-import-resolver-webpack": "^0.9.0",
"eslint-plugin-import": "^2.23.2",
"eslint-plugin-jsx-a11y": "^6.4.1",
"eslint-plugin-prettier": "^2.3.1",
"eslint-plugin-react": "^7.23.2",
"istanbul-instrumenter-loader": "^3.0.0",
"karma": "^6.3.6",
"karma-chrome-launcher": "2.2.0",
"karma-coverage": "^2.0.3",
"karma-firefox-launcher": "1.0.1",
"karma-mocha": "^2.0.1",
"karma-mocha-reporter": "^2.2.1",
"karma-sourcemap-loader": "^0.3.8",
"karma-webpack": "^2.0.4",
"mocha": "^8.4.0",
"npm-run-all": "^4.1.1",
"prettier": "^1.19.1",
"esbuild": "0.20.0",
"esbuild-dev-server": "^0.3.0",
"esbuild-node-externals": "^1.13.0",
"eslint": "^8.56.0",
"eslint-plugin-react": "^7.33.2",
Copy link

Choose a reason for hiding this comment

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

This diff is amazing! 😂

@diasbruno
Copy link
Collaborator Author

diasbruno commented Feb 27, 2024

If the module distributes just the minified version of the library, wouldn't the entrypoint file still need to use ESM module syntax (or equivalent)?

Depends on which version of the language (ES5, ES2015, ...) we compile it to.

If we don't pre-compile the library, users needs to have all the correct languages features to compile (classes, arrows and so on). But, if we move away from class components - I'm guessing, it will no longer be a problem.

I'm thinking on release this as v4-rc1, so we can give it a try and see how that works.

@diasbruno
Copy link
Collaborator Author

I'm going to drop esm/cjs for the first release candidate, so we can test it early.

@doeg
Copy link

doeg commented Feb 28, 2024

Makes sense to me! Let me know if I can be helpful in testing or otherwise. 😸

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Moving to a "modern" build system (hardcore level)
2 participants