rollup / rollup-plugin-commonjs

This module has moved and is now available at @rollup/plugin-commonjs / https://github.com/rollup/plugins/blob/master/packages/commonjs
MIT License
502 stars 126 forks source link

`preferBuiltins: false` has no effect, suggests setting `preferBuiltins` #392

Closed mikemaccana closed 4 years ago

mikemaccana commented 5 years ago

I am attempting to use the assert module from npm. preferBuiltins: false has no effect in either rollup-plugin-commonjs or rollup-plugin-node-resolve.

Using the following rollup,config.js


// From app - https://svelte.dev/blog/the-easiest-way-to-get-started
import commonjs from 'rollup-plugin-commonjs'
import svelte from 'rollup-plugin-svelte';
import nodeResolve from 'rollup-plugin-node-resolve';
import livereload from 'rollup-plugin-livereload';

const production = !process.env.NODE_ENV;

export default {
  input: 'src/frontend/index.js',
  output: {
    file: 'public/js/bundle.js',
    format: 'iife',
    name: 'applessBundle',
    sourcemap: true
  },
  plugins: [
    svelte({
      // enable run-time checks when not in production
      dev: !production,
      // we'll extract any component CSS out into
      // a separate file  better for performance
      css: css => {
        css.write('public/css/bundle.css');
      },
      // Warnings are normally passed straight to Rollup. You can
      // optionally handle them here, for example to squelch
      // warnings with a particular code
      onwarn: (warning, handler) => {
        // e.g. don't warn on autofocus
        if (warning.code === 'a11y-autofocus') {
          return;
        }

        // let Rollup handle all other warnings normally
        handler(warning);
      }
    }),

    // If you have external dependencies installed from
    // npm, you'll most likely need these plugins. In
    // some cases you'll need additional configuration 
    // consult the documentation for details:
    // https://github.com/rollup/rollup-plugin-commonjs
    nodeResolve({
      preferBuiltins: false     
    }),
    commonjs({
      include: /node_modules/,
      preferBuiltins: false         
    }),

    // Watch the `public` directory and refresh the
    // browser on changes when not in production
    !production && livereload('public')
  ]
}

Produces the following error:

(!) node-resolve plugin: preferring built-in module 'assert' over local alternative at 'C:\Users\mikem\Code\appless\node_modules\assert\assert.js', pass 'preferBuiltins: false' to disable this behavior or 'preferBuiltins: true' to disable this warning                              

(!) commonjs plugin: preferring built-in module 'assert' over local alternative at 'C:\Users\mikem\Code\appless\node_modules\assert\assert.js', pass 'preferBuiltins: false' to disable this behavior or 'preferBuiltins: true' to disable this warning      
joeyhub commented 5 years ago

It works for me if I put it in resolve only (I have no options for commonjs and I'm using the API).

Though I have all other kinds of hell after that. Using import export means things that you import that are commonjs get wrapped under a global use strict so basically stuff isn't going to work. To add insult to injury it tries to include things like util but doesn't because of a circular reference, so the built script tries to use util then dies because its not defined.

You could also try with the builtins plugin. However I've had trouble with that as well inserting unused shims that crash.

I personally see it as a lost cause trying to import assert from npm. Instead I have a plugin that imports dynamically based on target and file extension (IE, checking for _backend and _frontend suffixes).

shellscape commented 4 years ago

Hey folks (this is a canned reply, but we mean it!). Thanks to everyone who participated in this issue. We're getting ready to move this plugin to a new home at https://github.com/rollup/plugins, and we have to do some spring cleaning of the issues to make that happen. We're going to close this one, but it doesn't mean that it's not still valid.

We've got some time yet before the move while we resolve pending Pull Requests, so if this issue is still relevant, please @ me and I'll make sure it gets reopened and transferred to the new repo. :beer: