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

Use rollup-plugin-node-polyfills instead of node-globals+node-builtins #11098

Closed

Conversation

nicolo-ribaudo
Copy link
Member

Q                       A
Fixed Issues? Unblocks #10747
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes? Yes
License MIT

#10747 is currently blocked by calvinmetcalf/rollup-plugin-node-globals#34, which is using an old acorn version which doesn't support Node 10 features.

There are three different PRs to update acorn in that package:

However, the last release was in Sept 2018.

In this PR, I am replacing it with rollup-plugin-node-polyfills: it's a fork of rollup-plugin-node-builtins (used for built-in modules) which also polyfills global Node variables, maintained by the author of the second PR I linked.

It seems that the readme is still a copy of rollup-plugin-node-builtins, but looking at the source code and at rollup/rollup#2881 it seems to be exactly what we want.

PS. @manucorporat Are there any plans to move the plugin to https://github.com/rollup/plugins? Your plugin seems to be a fundamental piece of the ecosystem, and it would make sense to move it to the rollup org.

@nicolo-ribaudo nicolo-ribaudo added the PR: Internal 🏠 A type of pull request used for our changelog categories label Feb 5, 2020
yarn.lock Outdated
magic-string@^0.25.2:
version "0.25.4"
resolved "https://registry.yarnpkg.com/magic-string/-/magic-string-0.25.4.tgz#325b8a0a79fc423db109b77fd5a19183b7ba5143"
integrity sha512-oycWO9nEVAP2RVPbIoDoA4Y7LFIJ3xRYov93gAyJhZkET1tNuB0u7uWkZS2LpBWTJUWnmau/To8ECWRC+jKNfw==
dependencies:
sourcemap-codec "^1.4.4"

magic-string@^0.25.3:
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Can you run yarn-deduplicate?

@nicolo-ribaudo
Copy link
Member Author

I'll merge this not to master but to next-8-dev, because rollup-plugin-node-polyfills doesn't support node 6.

@nicolo-ribaudo nicolo-ribaudo changed the base branch from master to next-8-dev February 6, 2020 15:07
@nicolo-ribaudo
Copy link
Member Author

I have cherry-picked this PR to #10747, since they depend on each other to pass CI.

@nicolo-ribaudo nicolo-ribaudo deleted the rollup-inject-polyfills branch February 6, 2020 15:13
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label May 8, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Internal 🏠 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants