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

Externalize dependencies in esm bundle #12

Merged
merged 1 commit into from
Dec 26, 2024

Conversation

bluwy
Copy link
Contributor

@bluwy bluwy commented Dec 25, 2024

Not sure if it's intentional, but since emoji-regex-xs, regex and regex-recursion is already part of the dependencies list, the bundled code could externalize them to import the packages as is.

@slevithan
Copy link
Owner

Thanks for the PR. It is intentional, but I'm open to feedback/change.

What are the benefits to externalizing? Is it simply that code that uses e.g. both Oniguruma-To-ES and Regex+ wouldn't duplicate any code? (Note that the bundled code from dependencies is fairly small, since Oniguruma-To-ES imports only regex/internals from Regex+.)

The advantage of the current setup is that the ESM bundle can be used directly in browsers.

@bluwy
Copy link
Contributor Author

bluwy commented Dec 25, 2024

The dependencies are installed together when installing oniguruma-to-es, so it seems a bit off to me to not use them in the published code. If it's intended to bundle everything, I think the package should've moved those dependencies as devDependencies. If not, the developer would be installing duplicated/unused code.

But I think it's usually better to not bundle dependencies and have a setup like this PR. It's usually how npm packages are built. About browser support, I think the "browser" field points to a minified JS (which is not externalizing dependencies) so it's fine? Also for ESM entrypoints, CDNs like https://esm.sh can also handle externalized dependencies.

@slevithan slevithan merged commit f24d254 into slevithan:main Dec 26, 2024
2 checks passed
@bluwy bluwy deleted the externalize-deps branch December 26, 2024 23:01
@slevithan
Copy link
Owner

Released in v0.9.0. 😊

@bluwy
Copy link
Contributor Author

bluwy commented Dec 27, 2024

Thanks!

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.

2 participants