calvinmetcalf / rollup-plugin-node-builtins

138 stars 40 forks source link

Uncaught TypeError: Cannot read property 'prototype' of undefined #31

Open binarykitchen opened 7 years ago

binarykitchen commented 7 years ago

Happens when util.inherits(Duplex, Readable); is executed, see screenshot:

selection_009

To reproduce, just git clone https://github.com/binarykitchen/videomail-client/tree/feature/rollupjs and run yarn examples. Then you see that in the console.

calvinmetcalf commented 7 years ago

considering that the line is inherits(Duplex, Readable); I think you're issue is that you aren't actually using this library

binarykitchen commented 7 years ago

hmmm, if you think i am not using it, how can i exclude it then?

calvinmetcalf commented 7 years ago

that is a more general rollup question, there might be a module replacer plugin or something that might rewrite readable-stream calls to just regular stream calls

binarykitchen commented 7 years ago

ugh, that's probably new / out of my league. hints or examples how to replace this would be very welcome

calvinmetcalf commented 7 years ago

maybe https://github.com/rollup/rollup-plugin-alias or https://github.com/Hedzer/rollup-plugin-import-alias

binarykitchen commented 7 years ago

@calvinmetcalf ok, what exactly would you replace with that alias plugin?

    alias({
      'Readable': 'what to put here??' <-----
    })
calvinmetcalf commented 7 years ago

readable-stream:'stream' probably

binarykitchen commented 7 years ago

@calvinmetcalf thanks but i am afraid, i am seeing this error with that change:

(670): node_modules/websocket-stream/stream.js🚨   Could not load stream (imported by /home/michael-heuberger/code/videomail-client/node_modules/websocket-stream/stream.js): ENOENT: no such file or directory, open 'stream'

for reference, this is the whole config:

export default {
  entry: 'src/index.js',
  moduleName: uppercamelcase(moduleName),
  sourceMap: 'inline',
  plugins: [
    replace({'process.env.NODE_ENV': JSON.stringify('development')}),
    progress(),
    stylusCssModules({output: false}),
    json({preferConst: true}),
    alias({
      'readable-stream': 'stream'
    }),
    commonjs(),
    buble(),
    builtins(),
    resolve({jsnext: true, browser: true}),
    globals(),
    filesize({showGzippedSize: false})
  ],
  targets: [{
    dest: packageInfo.main,
    format: 'iife'
  }]
}

not sure if the alias plugin should be moved further down?

tex0l commented 7 years ago

Apparently it's because rollup-plugin-alias resolves the path of the plugin, I've opened an issue about it since I run into the exact same issue... https://github.com/rollup/rollup-plugin-alias/issues/33

For the time being I fork the packages I need and I replace 'readable-stream' with 'stream' manually...

binarykitchen commented 7 years ago

have switched to webpack i am afraid ...

CMCDragonkai commented 6 years ago

I'm seeing this error too. I'm also not using this package explicitly. My offending error is at:

util$3.inherits(Duplex$1, _stream_readable);

I'm using readable-stream as a dependency. And this is my rollup producing a browser target.

  {
    input: 'lib/index.js',
    output: {
      file: 'dist/index.browser.umd.js',
      format: 'umd',
      name: 'virtualfs'
    },
    plugins: [
      babel({
        babelrc: false,
        exclude: 'node_modules/**',
        runtimeHelpers: true,
        plugins: ['transform-object-rest-spread', 'transform-runtime', 'transform-class-properties'],
        presets: [
          'flow',
          ['env', {
            modules: false,
            targets: {
              browsers: ['last 2 versions']
            }
          }]
        ]
      }),
      resolve({
        preferBuiltins: false,
        browser: true
      }),
      commonjs({
        namedExports: {
          'node_modules/process/browser.js': ['nextTick']
        }
      })
    ]
  }
CMCDragonkai commented 6 years ago

I just ended up finding related discussion in https://github.com/nodejs/readable-stream/issues/237 So it seems I'm supposed to not use readable-stream at all. But I want to maintain stability for both node users and browser users.

If I do use this to resolve the rollup issue with readable-stream how does this interact with other polyfills that I already have? I have existing polyfills for buffer, process, path, events... etc. I would only want this to fix the stream issue.

calvinmetcalf commented 6 years ago

it might interact weirdly, with other polyfills, but based on your config, you only have one for next tick

CMCDragonkai commented 6 years ago

The rest of my polyfills are in the dev dependencies, since they are only brought in wheb bundling with rollUp.

On 15 May 2018 00:28:55 GMT+10:00, Calvin Metcalf notifications@github.com wrote:

it might interact weirdly, with other polyfills, but based on your config, you only have one for next tick

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/calvinmetcalf/rollup-plugin-node-builtins/issues/31#issuecomment-388837183

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

calvinmetcalf commented 6 years ago

so this includes polyfills as well, so it could case some weird issues if you try to include both

On Mon, May 14, 2018 at 2:36 PM Roger Qiu notifications@github.com wrote:

The rest of my polyfills are in the dev dependencies, since they are only brought in wheb bundling with rollUp.

On 15 May 2018 00:28:55 GMT+10:00, Calvin Metcalf < notifications@github.com> wrote:

it might interact weirdly, with other polyfills, but based on your config, you only have one for next tick

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub:

https://github.com/calvinmetcalf/rollup-plugin-node-builtins/issues/31#issuecomment-388837183

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/calvinmetcalf/rollup-plugin-node-builtins/issues/31#issuecomment-388840017, or mute the thread https://github.com/notifications/unsubscribe-auth/ABE4n1iVX1cegyEmax03DLqYUKfJxkUyks5tyZaAgaJpZM4OENl_ .

CMCDragonkai commented 6 years ago

I ended converting to just using your polyfills @calvinmetcalf and switching from readable-stream to stream. I hope everything works.