search

reactjs / react-modal

Accessible modal dialog component for React
http://reactcommunity.org/react-modal
MIT License
7.34k stars 807 forks source link

WIP: rework build system to use esbuild. #1043

Open diasbruno opened 4 months ago

diasbruno commented 4 months ago

Fixes #1036.

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

It need to:

diasbruno commented 4 months ago

@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 commented 4 months ago

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 commented 4 months ago

@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)?

diasbruno commented 4 months ago

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 commented 4 months ago

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

doeg commented 4 months ago

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