Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamic Import - Error: Cannot split a chunk that has already been edited #2660

Closed
jleveugle opened this issue Jan 18, 2019 · 7 comments · Fixed by #2664
Closed

Dynamic Import - Error: Cannot split a chunk that has already been edited #2660

jleveugle opened this issue Jan 18, 2019 · 7 comments · Fixed by #2664

Comments

@jleveugle
Copy link

  • Rollup Version: 1.1.0
  • Operating System (or Browser): Mac OS X
  • Node Version: 10.13.0

How Do We Reproduce?

Hello,

I created an example here to explain / isolate the issue: https://github.com/jleveugle/rollup-nested-dynamic-imports

I hope it will be understandable :)

In fact, if you try to play with nested dynamic imports, with one of them that is rewritten by a plugin (using magic-string, or something else), your build will failed with rollup@^1.0, while it worked with rollup@^0.66 :)

In my example, I import a fake "bar" (named baar), and my plugin replace baar by bar by using a dynamic import.

Expected Behavior

Build should success, as with rollup@^0.66:

➜  rollup-nested-dynamic-imports git:(rollup-0.66) npm start

> rollup-nested-dynamic-imports@0.0.0 start /Users/jleveugle/Documents/Work/github/jleveugle/rollup-nested-dynamic-imports
> rollup -c


index.js → ./dist...
created ./dist in 19ms

Actual Behavior

Build fails, with an error from magic-string.

➜  rollup-nested-dynamic-imports git:(master) npm start
$ rollup -c

index.js → ./dist...
[!] Error: Cannot split a chunk that has already been edited (1:18 – "import('/Users/jleveugle/Documents/Work/github/jleveugle/rollup-nested-dynamic-imports/bar')")
Error: Cannot split a chunk that has already been edited (1:18 – "import('/Users/jleveugle/Documents/Work/github/jleveugle/rollup-nested-dynamic-imports/bar')")
    at MagicString._splitChunk (/Users/jleveugle/Documents/Work/github/jleveugle/rollup-nested-dynamic-imports/node_modules/rollup/dist/rollup.js:1692:9)
    at MagicString._split (/Users/jleveugle/Documents/Work/github/jleveugle/rollup-nested-dynamic-imports/node_modules/rollup/dist/rollup.js:1682:46)
    at MagicString.overwrite (/Users/jleveugle/Documents/Work/github/jleveugle/rollup-nested-dynamic-imports/node_modules/rollup/dist/rollup.js:1448:7)
    at Import.renderFinalResolution (/Users/jleveugle/Documents/Work/github/jleveugle/rollup-nested-dynamic-imports/node_modules/rollup/dist/rollup.js:11985:18)
    at Chunk.finaliseDynamicImports (/Users/jleveugle/Documents/Work/github/jleveugle/rollup-nested-dynamic-imports/node_modules/rollup/dist/rollup.js:15202:30)
    at Chunk.render (/Users/jleveugle/Documents/Work/github/jleveugle/rollup-nested-dynamic-imports/node_modules/rollup/dist/rollup.js:15786:14)
    at /Users/jleveugle/Documents/Work/github/jleveugle/rollup-nested-dynamic-imports/node_modules/rollup/dist/rollup.js:18057:38
    at Array.map (<anonymous>)
    at /Users/jleveugle/Documents/Work/github/jleveugle/rollup-nested-dynamic-imports/node_modules/rollup/dist/rollup.js:18055:47
    at <anonymous>

error Command failed with exit code 1.

Thanks for your feedback,

@arxpoetica
Copy link

arxpoetica commented Jan 19, 2019

I can confirm that this works for me in 0.67.4 but not in anything including and past 0.68.0. (I'm using Rollup in Sapper / Svelte.)

@arxpoetica
Copy link

For what it's worth, here's my config: https://gist.github.com/arxpoetica/b71700a4893a43e3eabf45810ce4dd6b

@arxpoetica
Copy link

arxpoetica commented Jan 19, 2019

One more clue. I'm realizing this happens when I import { something } from 'somewhere' in one place, and then const { something } = await import('somewhere') in another place.

@lukastaegert
Copy link
Member

Fix at #2664. Thanks so much, this bug was fairly simple to resolve but had some profound consequences.

@jleveugle
Copy link
Author

Hello @lukastaegert,

Thanks for the fix! Next time, I hope I can do more than one issue ;)

Honestly, I was trying to debug on my side, but the time to familiarize myself with the codebase, the fix didn't seem so easy ^.^

Glad to have helped the project and thanks again :)

@lukastaegert
Copy link
Member

lukastaegert commented Jan 21, 2019

No need to worry, the fix looks simple but there was quite a bit of indirection—and familiarity with the algorithm—involved to find it. The thing is the actual issue is that if a dynamic import is chunked together with another module that imports it as well as being dynamically imported by a third module that ends up in another chunk, then rollup tries to rewrite the import statement in two different ways at the same time—which of course fails.

The thing is, this situation should never be possible to happen which lead me to investigate why it was happening and finally to the code in the chunk coloring module.

However the core issue described is still not fixed. If in the future e.g. we open up the chunking algorithm to allow arbitrary chunk creation, then it will still be possible to recreate this situation and then we really need to fix it, which will be quite a bit more work.

In a way, I am glad this caused an issue because otherwise it would have taken us a really long time to even discover that the chunking was broken in such a fundamental way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants