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

expose lite build without dependencies bundled #105

Merged
merged 5 commits into from May 4, 2020

Conversation

nikaspran
Copy link
Contributor

This is a first pass at creating a new build artifact without the estraverse dependency being bundled. See #104 for details.

Fixes:

Allows workarounds for:

I've left the current main entrypoint as esquery.min.js, but it might make sense to make lite the entry point instead.

Alternatively, if the current approach in the PR is too messy, I could alter it instead to make this a simple npm module alongside the build (so if required via Node, it would use the source and regular requires, but the dist would still be there for direct usage). I would wager a guess that the majority use case for this library is via Node, so this shouldn't impact a large amount of people, but would still probably necessitate a breaking change semver update.

Let me know your thoughts and where I should take this PR.

P.S.: Love the library! ❤️

@brettz9
Copy link
Contributor

brettz9 commented Apr 30, 2020

Nice to see a backward-compatible solution which doesn't remove the browser-friendly ESM build (prebuilt "binaries" are useful for the browser too!).

One could technically add lite ESM builds too for use in more recent module-supporting Node versions (but since even import can be used with CJS (with a default export, as in esquery), there is less need for this, I think).

@nikaspran
Copy link
Contributor Author

@brettz9 good point, added the lite esm build as well. Let me know if there's anything else missing before this can get merged in!

@brettz9
Copy link
Contributor

brettz9 commented Apr 30, 2020

I'm afraid that the way Node requires type or .mjs for modules, this would also require adding the .mjs extension to the dist file (or adding type: "module" to package.json (though the latter would require other refactoring).

Btw, you might be interested in the discussion at estools/esutils#34 .

@nikaspran
Copy link
Contributor Author

Hmmm, fair enough, forgot about that. Somehow the .mjs extension seems a bit too Node oriented and also would lead to inconsistent artifacts, i.e.:

dist/esquery.esm.js  

// BUT

dist/esquery.lite.mjs

So maybe the best option would be to just roll back the previous commit and stick with one lite build for umd for now (since Node supports it anyhow)?

@brettz9
Copy link
Contributor

brettz9 commented Apr 30, 2020

Not my call, but it'd be fine with me with the "mjs" extension. It would only make sense for Node (or compilers) in such a build at this point anyways, given that browsers (sans import maps at least) couldn't make sense of the npm specifiers.

And from a forward-looking perspective, servers can serve "mjs" as JS to browsers already too, some like Github Pages already do, and one might argue it is even the best extension to use for polyglot JS files now as it avoids Node needing to check package.json for the type).

rollup.config.js Outdated Show resolved Hide resolved
rollup.config.js Outdated Show resolved Hide resolved
@michaelficarra michaelficarra changed the title Expose lite build without estraverse being bundled expose lite build without dependencies bundled May 4, 2020
@michaelficarra michaelficarra merged commit 6a0b228 into estools:master May 4, 2020
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.

None yet

3 participants