Stop recommending node-globals/node-builtins plugin

[manucorporat]

The docs recommend users to use rollup-plugin-node-builtins and rollup-plugin-node-globals if you code uses node APIS, but they are pretty much unmaintained.

For example, rollup-plugin-node-globals uses a very old version of acorn instead of this.parse() provided by rollup, making any build including a dynamic import() to crash.

I would be great if the rollup organization forked this plugins or find some alternatives.
We have submitted a couple of PRs fixing this plugins (since december of 2018), but they are not being merged.

[lukastaegert]

Good point. Maybe we should point people to rollup-plugin-inject instead, on which this plugin was based, and develop a meta-plugin that basically configures rollup-plugin-inject to inject node globals. The issue, however, is maintenance. As you have obviously put some thought into this, would you be willing to help with maintenance (i.e. checking issues, updating dependencies, I would grant you the necessary permissions to do so) if I set something up within the rollup organization?

[manucorporat]

Sorry for taking so long, it's been a busy week :)
Yeah! I am currently working in exactly that:

• combine builtins and globals
• using rollup-inject under the hood!
• probably name it: rollup-plugin-node-polyfills

I will keep you tuned!

[lukastaegert]

Awesome! If you want to make it an official plugin at some point, let me know.

[manucorporat]

@lukastaegert
https://github.com/ionic-team/rollup-plugin-node-polyfills/tree/master/src

• Relies on inject for global patching
• Copied builtins from rollup-plugin-node-builtins
• Ported to TS for an extra of safety and type definitions

Open questions, wondering if we should rollup the following dependencies:

    "browserify-fs": "^1.0.0",
    "buffer-es6": "^4.9.2",
    "crypto-browserify": "^3.11.0",
    "process-es6": "^0.11.2",

to avoid the dependency hell this packages could introduce.

[manucorporat]

I would be happy to maintain this plugin and make the rollup organisation adopt it.

[lukastaegert]

Ported to TS for an extra of safety and type definitions

Nice one! I was wondering if I should suggest it considering the long term plan would be to move more and more of the Rollup ecosystem to TS but it seems you beat me to it 😉

Open questions, wondering if we should rollup the following dependencies

Does not look like there would be lots of synergies with other plugins but who knows what transitive dependencies there are. As for licenses and dependencies, this is a nice tool: https://npm.anvaka.com/#/view/2d/browserify-fs

Looks like including buffer-es6 and process-es6 should be no-brainers though technically if we bundle them, we should make sure there licenses are reflected in our license (I guess we should do this for Rollup as well, I do not have too much experience with this to be honest. But most of these licenses require you to distribute a copy of the license together with the source code in some form.)

I am a little cautious about the browserify shims, they seem to have TONS of dependencies, some with questionable licenses. One could definitely experiment with bundling, an alternative could be not NOT make them a dependency but instead tell the user that they need to install this dependency if they enable that shim. Then we could try to do a dynamic import and if it fails, tell the user what they need to install.

[lukastaegert]

Rollup is doing this with chokidar.

[eight04]

Don't use browserify-fs. It is unmaintained for years. It is built on top of a very old version/implementation of level-db which includes many outdated dependencies within vulnerability.

Some ideas:

1. Don't include large dependencies by default. (e.g. Buffer, fs)
2. Warn the user when these large modules are used in the bundle. (Don't warn if the dependency is tree-shaken, assumes that the builtin modules have no side-effects).
3. Allow users to define an alias table so they can use their own shim:

   polyfill({
     alias: {
       path: "path-browserify", // map to "path-browserify"
       buffer: "./src/shim/simple-buffer.js" // map to a module inside the `src/shim` folder
     }
   });

[manucorporat]

is there some alternative to browserify-fs? i am also a fan of avoiding it.

I got some nice bundling, making each dep a single .fs file, but I didn't though about the LICENSES.

I hate dependencies by the way.

[manucorporat]

I think i will updtae the deps to use what webpack uses. Which it's like the de facto standard https://github.com/webpack/node-libs-browser

[lukastaegert]

Sounds good. And don’t worry about licenses. I will try to fix this up for Rollup core the next days, there are some people at my company who should know a thing or two about FOSS licensing. Then we can think about what we need to write in the license for the bundled dependencies. But I think it is important we set a good precedent for these things as more libraries are starting to distribute bundled dependencies.

[manucorporat]

Sounds good. And don’t worry about licenses. I will try to fix this up for Rollup core the next days, there are some people at my company who should know a thing or two about FOSS licensing. Then we can think about what we need to write in the license for the bundled dependencies. But I think it is important we set a good precedent for these things as more libraries are starting to distribute bundled dependencies.

Thanks perfects! thanks for your time in this issue!
I think in this particular case, it makes a lot of sense to make most of the work of bundiling this shims on our side:

• some shims are commonjs (we can convert them to ESM, avoding users to add the commonjs plugin)
• improve build time (user's rollups will not have to walk though this massive shims everytime)
• improve install time
• better security auditory of what does the shim actually contain

Just released an initial version of it:

npm i rollup-plugin-node-polyfills --save-dev
import nodePolyfills from 'rollup-plugin-node-polyfills';

I am asking some users of stencil to use it in their apps, some had problems with dynamic imports, let's see how all this plays.

[bitflower]

Getting this in VS Code:

Type '{ name: string; resolveId(importee: string, importer: string): { id: any; moduleSideEffects: boolean; }; load(id: string): string; transform(code: string, id: string): any; }' is not assignable to type 'Plugin'.
  Types of property 'resolveId' are incompatible.
    Type '(importee: string, importer: string) => { id: any; moduleSideEffects: boolean; }' is not assignable to type '(importee: string, importer: string, context?: PluginCtx) => string | Promise<string>'.
      Type '{ id: any; moduleSideEffects: boolean; }' is not assignable to type 'string | Promise<string>'.
        Type '{ id: any; moduleSideEffects: boolean; }' is missing the following properties from type 'Promise<string>': then, catch, [Symbol.toStringTag], finally

Stencil 1.0.1
“rollup-plugin-node-polyfills”: “^0.1.2"

Good old rollup-plugin-node-builtins did the trick

[lukastaegert]

The error looks weird to me. Do you have any additional/external Rollup types installed?

Especially this part is strange: '(importee: string, importer: string, context?: PluginCtx) => string | Promise<string>'. Nowhere in the Rollup codebase is a type named PluginCtx. Could you find out where this type is defined?

[manucorporat]

@lukastaegert no worries, that error is related with stencil, we have some bad types, i will fix soon! unrelated with rollup :)

[lukastaegert]

As for the licensing, I hope we can extend rollup-plugin-license to gather all license texts in a single file to automatically include all dependencies without forcing us to manually maintain that file, see mjeanroy/rollup-plugin-license#379

[shellscape]

Hey folks. This is a saved-form message, but rest assured we mean every word. The Rollup team is attempting to clean up the Issues backlog in the hopes that the active and still-needed, still-relevant issues bubble up to the surface. With that, we're closing issues that have been open for an eon or two, and have gone stale like pirate hard-tack without activity.

We really appreciate the folks have taken the time to open and comment on this issue. Please don't confuse this closure with us not caring or dismissing your issue, feature request, discussion, or report. The issue will still be here, just in a closed state. If the issue pertains to a bug, please re-test for the bug on the latest version of Rollup and if present, please tag @shellscape and request a re-open, and we'll be happy to oblige.

[lukastaegert]

Reopening to track state of the new plugin

[shellscape]

@manucorporat I'd like to propose moving your plugin to the Rollup org. We think it's an important plugin and worth being backed by the org. Is that something you'd be cool with?

[shellscape]

@manucorporat ping. we're just about done with populating our new plugins monorepo and would like to bring this into the fold. Thoughts?

[carloscarvallo]

and what's the final recommendation for using nodejs utilities?

[Niek]

Since @manucorporat is MIA and https://github.com/ionic-team/rollup-plugin-node-polyfills is unmaintained (no commits for 1,5 year, no PRs merged) - what is the best approach/alternative?

[eight04]

alternative

I'm using the alias plugin to map those modules to our own implementation:
https://github.com/openstyles/stylus-lang-bundle/blob/3da0291a5b7877179a3e0990072489c59e7b0f06/rollup.config.js#L17-L26

[lukastaegert]

I guess we should just put a fork into the plugins monorepo and take it over from our side. Anyone willing to help here?

[pd4d10]

Encountered this problem +1. Is there anything I can help with?

[lukastaegert]

If @shellscape agrees as well, what would need to be done is to create a PR for https://github.com/rollup/plugins that

• adds this plugin
• updates the plugin to conform stylistically to the other plugins in the repo
  • use Ava tests
  • use pnpm for dependencies
  • use consistent linting
  • other things I am not aware of at the moment
• update the documentation of the repo to mention the plugin

I believe if someone starts with a PR that would just add the plugin as it is, more people can help with the additional steps needed to refine the PR. @shellscape would know more how to go about this best.

[yaquawa]

For those who only using the process.env.NODE_ENV, this would be the alternative.

https://www.npmjs.com/package/rollup-plugin-inject-process-env

[kuraga]

@yaquawa what do you mean?

[shellscape]

@lukastaegert that's a really, really big burden. a large portion of webpack's issues were driven by the node compat layer. this is one of those that I don't actually think should be in the plugins repo, but a separate fork under the guise of "we don't recommend using this, but if you really really have to, here it is" and let the community maintain it. To that end, maybe it should be its own project outside of the org.

[eight04]

How about adding a "Recommended polyfills for Node.js builtin modules" section to rollupjs.org or https://github.com/rollup/plugins/tree/master/packages/commonjs? Then rollup can point people to the list and the alias/inject plugin. (maybe add some examples to plugin readme or the doc)

[patak-dev]

@lukastaegert @shellscape we are having a lot of bug reports opened against the Vite repo because of the use of node globals. Looks like https://github.com/ionic-team/rollup-plugin-node-polyfills has not been updated lately. Do you have any insight on other projects that we could recommend?

Note: with Webpack 5 dropping the shims, I think popular dependencies will get patched soon, so this pain may be good for the ecosystem in the long run.

[Niek]

@matias-capeletto https://github.com/snowpackjs/rollup-plugin-polyfill-node is a good alternative and being maintained

See also: rollup/plugins#51 (comment)

[patak-dev]

Thanks a lot for the pointers @Niek, that are great news! 🎉

[curran]

Dropped a PR to recommend snowpackjs/rollup-plugin-polyfill-node instead of ionic-team/rollup-plugin-node-polyfills : #4190

[bumbummen99]

snowpackjs/rollup-plugin-polyfill-node has been deleted

[curran]

Oh no!

I see that it's still there on NPM at least https://www.npmjs.com/package/rollup-plugin-polyfill-node

I did some digging and found that:

• There is a lookalike repo at https://github.com/FredKSchott/rollup-plugin-polyfill-node
• The owner there is the same person that added it to snowpack in the first place, so I think it's legit update polyfill node rollup plugin FredKSchott/snowpack#2053

Related thread rollup/plugins#51 (comment)

[eduardoacskimlinks]

Hi, All; thanks for all your great work on this great project.

I am trying to upgrade an old repository from Rollup 1 to 4 (I know, I know, long overdue). I came across this post, but I don't have a clear direction on how to upgrade the following plugins:

• rollup-plugin-node-builtins
• rollup-plugin-node-globals

What npm/yarn package should I use to support these features? If it's no longer required, can you point me in the right direction within the docs?

Thanks in advance (I know this is not for an issue but more for a StackOverflow question), but I thought the subject of the original issue made my question relevant here. I apologise in advance 😉 if it wasn't the right place