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

plugin-legacy:Unexpected token import #2786

Closed
rashagu opened this issue Mar 30, 2021 · 8 comments · Fixed by #3023
Closed

plugin-legacy:Unexpected token import #2786

rashagu opened this issue Mar 30, 2021 · 8 comments · Fixed by #3023

Comments

@rashagu
Copy link

rashagu commented Mar 30, 2021

Describe the bug

I used the plugin @vitejs/plugin-legacy

legacy({
     targets: ['chrome 53']
}),

But the packaged code still contains import"core-js/modules/es.promise.js";

****-legacy.278fab9b.js:1 Uncaught SyntaxError: Unexpected token import
SyntaxError: Unexpected token import
****-legacy.278fab9b.js:1 Uncaught (in promise) SyntaxError: Unexpected token import

Reproduction

When some components are split into a single file
In fact, I can’t reproduce this bug with the least amount of code

System Info

System:
  OS: Windows 10 10.0.19042
  CPU: (8) x64 AMD Ryzen 5 2500U with Radeon Vega Mobile Gfx
  Memory: 998.52 MB / 6.90 GB
Binaries:
  Node: 14.15.3 - C:\Program Files\nodejs\node.EXE
  Yarn: 1.22.10 - ~\AppData\Roaming\npm\yarn.CMD
  npm: 6.14.9 - C:\Program Files\nodejs\npm.CMD
Browsers:
  Chrome: 87.0.4280.88
  Edge: Spartan (44.19041.423.0), Chromium (89.0.774.63)
  Internet Explorer: 11.0.19041.1
npmPackages:
  @vitejs/plugin-vue: ^1.1.5 => 1.2.0
  vite: ^2.1.4 => 2.1.4
@vibo
Copy link

vibo commented Apr 1, 2021

Having the same issue in a large react project with code splitting targeting IE 11.
Also unable to reproduce doing similar splitting on a smaller scale.
Can note that it works for us if you remove these imports manually.

@fnlctrl
Copy link
Contributor

fnlctrl commented Apr 12, 2021

Ran into this as well. It only occured after the project got bigger in size.

Fiddled around and it seems even if recordAndRemovePolyfillBabelPlugin removed all ImportDeclarations babel still added import"core-js/modules/es.promise.js"; at the beginning...

@TobiasMelen
Copy link
Contributor

TobiasMelen commented Apr 12, 2021

I'm having es imports in the legacy chunk as well. For me, entry legacy chunk starts with import"core-js/modules/es.array.from.js";import"core-js/modules/es.symbol.iterator.js";, so it's not necessarily directly related to the promise-polyfill.
The if statement for import trimming in recordAndRemovePolyfillBabelPlugin is not entered for these imports. I have not been able to reproduce in a smaller, shareable repo.
Could Babel's syntax parsing miss the import? 🤷

Update. I did some further debugging and the imports are cleared if recordAndRemovePolyfillBabelPlugin is moved to a separate babel render/transform pass right after the one it's currently in. This makes little sense though, since presets are supposed to be ordered.
Could Babel 'preset-env' be secretly asynchronous? 🤷
(I mean kind of, but not really)

@TobiasMelen
Copy link
Contributor

Ok, so i found a way to fix it in our project. Moving the recording and removing of polyfills from Program:exit to post in the plugin lifecycle seems let all nodes finish processing before it's executed. The theory being that while the custom plugins Program:exit is run after @babel/preset-envs Program:exit there might still be AST-nodes left to process adding polyfills. https://jamie.build/babel-plugin-ordering.html

Since this is hard to reproduce it would be great with proof this actually fixes the issue before PR. @fnlctrl, @vibo and/or @rashagu: Could you try adding https://github.com/TobiasMelen/vite/blob/record-remove-polyfills-post/packages/plugin-legacy/index.js to a seperate js file and importing the plugin from that local file instead and then test if builds strip all import statements?

@fnlctrl
Copy link
Contributor

fnlctrl commented Apr 16, 2021

@TobiasMelen just tested and it works👍

@mis98zb
Copy link

mis98zb commented Apr 16, 2021

@TobiasMelen I have tried to build with the new code https://github.com/TobiasMelen/vite/blob/record-remove-polyfills-post/packages/plugin-legacy/index.js with my project, it works perfect!
Thank you for the fix!!!

the version of plugins is as below:
"vue": "^3.0.7",
"@vitejs/plugin-legacy": "^1.3.2",
"@vitejs/plugin-vue": "^1.1.5",
"@vue/compiler-sfc": "^3.0.7",
"vite": "^2.1.2"

@Shinigami92
Copy link
Member

Can this issue be closes? (And the other #2781 too)? Or do we need something in a PR?

@TobiasMelen
Copy link
Contributor

Needs PR. Since it seems to solve the issue i will add a one when i can, probably later today.

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

Successfully merging a pull request may close this issue.

7 participants