pmowrer / rollup-plugin-peer-deps-external

Automatically externalize `peerDependencies` in a rollup bundle.
MIT License
109 stars 16 forks source link

Doesn't work on rollup >= 2.11.0 #29

Closed tarsisexistence closed 4 years ago

tarsisexistence commented 4 years ago

Last well-compatible rollup version is 2.10.9

It throws an error because external arg is undefined in this function

export default function externalToFn(external) {
  if (isFunction(external)) {
    return external;
  } else if (Array.isArray(external)) {
    return id => external.some(module => module === id);
  } else {
    throw new Error(
      `rollup-plugin-peer-deps-external: 'external' option must be a function or an array.`
    );
  }
}

UPD: if I pass property to the rollup config external, then It works as usual

{
  external: () => false,
  plugins: [
     external(),
  ]
}
pmowrer commented 4 years ago

Thanks for the report @maktarsis! I'm unable to reproduce this with rollup 2.13.1. If I don't set external, it's an array by default. Any ideas why it might be undefined in your case?

Either way, might as well allow undefined... additionally, we're not allowing string/RegExp, which should be accepted per the docs. I'll have an update soon.

tarsisexistence commented 4 years ago

@pmowrer it's weird I get that error If I don't set external :(

I've no idea why this works like this, but I tested this behavior several times between rollup 2.10.9 and 2.11.0 (just tested on 2.13.1 and still getting error).

pmowrer commented 4 years ago

What other rollup plugins are you using? Is it possible something else is setting external to undefined?

tarsisexistence commented 4 years ago

Hmmm, I'm not sure, because it occurs only when I bump rollup to >=2.11.0 without touching any plugin.

using the following plugins:

import commonjs from '@rollup/plugin-commonjs';
import resolve from '@rollup/plugin-node-resolve';
import url from '@rollup/plugin-url';
import image from '@rollup/plugin-image';
import json from '@rollup/plugin-json';
import beep from '@rollup/plugin-beep';
import babel from '@rollup/plugin-babel';
import typescript2 from 'rollup-plugin-typescript2';
import postcss from 'rollup-plugin-postcss';
import stripCode from 'rollup-plugin-strip-code';
import filesize from 'rollup-plugin-filesize';
import progress from 'rollup-plugin-progress';
import autoprefixer from 'autoprefixer';
import cssnano from 'cssnano';
import simplevars from 'postcss-simple-vars';
import nested from 'postcss-nested';

Interesting 🤔

raphael22 commented 4 years ago

Same with this here:

import peerDepsExternal from 'rollup-plugin-peer-deps-external';
import commonjs from '@rollup/plugin-commonjs';
import resolve from '@rollup/plugin-node-resolve';
import babel, { getBabelOutputPlugin } from '@rollup/plugin-babel';
import { terser } from 'rollup-plugin-terser';
import visualizer from 'rollup-plugin-visualizer';
Nightbr commented 4 years ago

Same here using stenciljs v1.15.0 (Rollup 2.18.0).

[ ERROR ]  Rollup
           rollup-plugin-peer-deps-external: 'external' option must be a function or an array.

stencil.config.ts

import { sass } from '@stencil/sass';
import { postcss } from '@stencil/postcss';
import peerDepsExternal from 'rollup-plugin-peer-deps-external';

...
  plugins: [
    peerDepsExternal(),
    sass({
      injectGlobalPaths: ['src/globals/components.globals.scss'],
    }),
    postcss({
      plugins: [autoprefixer()],
    }),
...
pmowrer commented 4 years ago

Thanks for the reports and the patience! I'll have this fixed today!

pmowrer commented 4 years ago

:tada: This issue has been resolved in version 2.2.3 :tada:

The release is available on:

Your semantic-release bot :package::rocket: