-
Notifications
You must be signed in to change notification settings - Fork 214
[Sapper #1151] Updated rollup dependencies #221
Conversation
Note that It should probably be 'strict' in this instance. |
@arxpoetica @Conduitry I think we can just merge this and keep the commonjs version locked. This is how I have it set up in my own Sapper projects, and it works. FWIW I can't figure out what is happening with the issue in rollup, it appears to have been "fixed" but still doesn't work, and anybody asking when it will work receives a gif as an answer. I can't make head nor tail of it, maybe somebody else understands what needs to be done? |
A bit of further investigation seems to indicate |
It looks like @rollup/plugin-commonjs 12.0.0 is out now |
@@ -2,7 +2,7 @@ import resolve from '@rollup/plugin-node-resolve'; | |||
import replace from '@rollup/plugin-replace'; | |||
import commonjs from '@rollup/plugin-commonjs'; | |||
import svelte from 'rollup-plugin-svelte'; | |||
import babel from 'rollup-plugin-babel'; | |||
import babel from '@rollup/plugin-babel'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this move to the top to be alphabetized?
Can confirm that @arxpoetica if you don't mind upgrading the library in this PR, I believe it's ready to merge. |
Also update rollup, as the commonjs plugin has an updated peerDep.
w00t! Thanks for taking initiative on this @Conduitry @benmccann @antony I was asleep at the wheel :P |
I think #225 can be closed due to this PR. |
This is not ready for merging until an issue for
@rollup/plugin-commonjs
is fixed, which should be any day: rollup/plugins#304Before merging this PR should update that dependency to the fix first.
Relevant Sapper issue: sveltejs/sapper#1151
Will also fix: #218