Update to rollup 3.0 #62
Conversation
| @@ -10,7 +10,9 @@ export default { | |||
| dir: `dist/${format==='es' ? 'es' : ''}`, | |||
| exports: 'auto', | |||
| format, | |||
| preferConst: true, | |||
| generatedCode: { | |||
There was a problem hiding this comment.
preferConst is deprecated in rollup3. generatedCode.constBindings is the new way of doing it
| @@ -78,6 +78,7 @@ export default function (opts: NodePolyfillsOptions = {}): Plugin { | |||
| }, | |||
| transform(code: string, id: string) { | |||
| if(id === PREFIX + 'global.js') return | |||
| // @ts-ignore | |||
| return injectPlugin.transform!.call(this, code, id.replace(PREFIX, resolve('node_modules', 'polyfill-node'))); | |||
There was a problem hiding this comment.
TypeScript was complaining about injectPlugin.transform.call being not callable 😬
I suspect rollup updated the types behind their plugins and disallow a plugin to call an other plugin hook 🤷
There was a problem hiding this comment.
Then this might not be a good idea / they may have done this for a reason. @lukastaegert is this okay to ignore?
There was a problem hiding this comment.
@StoneCypher thanks for the ping! If it's been a 3+ days and Lukas hasn't responded here, someone please re-ping me and we can get this merged/released anyway and then see if any users run into issues that way.
There was a problem hiding this comment.
Sorry, missed the original ping. Rollup has no issue with people calling plugin hooks directly if they think they know what they are doing. The reason the types fail is because since recent Rollup 2 releases, plugin hooks can also be objects with a handler property that is the actual hook. Of course the inject plugin does not do that (yet?), so your solution should still work. @ts-ignore is a broad hammer, though. as TransformHook (which is exported by Rollup) might be a more fine-grained way that does not hide any other type errors that may sneak into this line.
There was a problem hiding this comment.
That being said, we do not consider people calling hooks directly a first-class way of using Rollup plugins, which is why we do not consider this a breaking change, and there may be more breaking changes of that kind in the future.
|
|
This pull request updates dependencies and peerDependencies to enable rollup 3.
Either it's ok to go like that or rollup-plugin-license needs to be updated first