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

fix(build): allow dynamic import treeshaking when injecting preload #14221

Merged
merged 10 commits into from
Jun 6, 2024
84 changes: 83 additions & 1 deletion packages/vite/src/node/plugins/importAnalysisBuild.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ const preloadMarkerRE = new RegExp(preloadMarker, 'g')

const dynamicImportPrefixRE = /import\s*\(/

const dynamicImportTreeshakenRE =
/(\b(const|let|var)\s+(\{[^}.]+\})\s*=\s*await\s+import\([^)]+\))|(\(\s*await\s+import\([^)]+\)\s*\)(\??\.[^;[\s]+)+)|\bimport\([^)]+\)(\s*\.then\([^{]*?\(\s*\{([^}.]+)\})/g

function toRelativePath(filename: string, importer: string) {
const relPath = path.posix.relative(path.posix.dirname(importer), filename)
return relPath[0] === '.' ? relPath : `./${relPath}`
Expand Down Expand Up @@ -235,6 +238,66 @@ export function buildImportAnalysisPlugin(config: ResolvedConfig): Plugin {
return null
}

// when wrapping dynamic imports with a preload helper, Rollup is unable to analyze the
// accessed variables for treeshaking. This below tries to match common accessed syntax
// to "copy" it over to the dynamic import wrapped by the preload helper.
const dynamicImports: Record<
number,
{ declaration?: string; names?: string }
> = {}

if (insertPreload) {
let match
while ((match = dynamicImportTreeshakenRE.exec(source))) {
/* handle `const {foo} = await import('foo')`
*
* match[1]: `const {foo} = await import('foo')`
* match[2]: `const`
* match[3]: `{foo}`
* import end: `const {foo} = await import('foo')_`
* ^
*/
if (match[1]) {
dynamicImports[dynamicImportTreeshakenRE.lastIndex] = {
declaration: `${match[2]} ${match[3]}`,
names: match[3]?.trim(),
sun0day marked this conversation as resolved.
Show resolved Hide resolved
}
continue
}

/* handle `(await import('foo')).foo`
*
* match[4]: `(await import('foo')).foo`
* match[5]: `.foo`
* import end: `(await import('foo'))`
* ^
*/
if (match[4]) {
let names = match[5].match(/\.([^.?]+)/)?.[1] || ''
// avoid `default` keyword error
if (names === 'default') {
names = 'default: __vite_default__'
}
dynamicImports[
dynamicImportTreeshakenRE.lastIndex - match[5]?.length - 1
] = { declaration: `const {${names}}`, names: `{ ${names} }` }
continue
}

/* handle `import('foo').then(({foo})=>{})`
*
* match[6]: `.then(({foo}`
* match[7]: `foo`
* import end: `import('foo').`
* ^
*/
const names = match[7]?.trim()
dynamicImports[
dynamicImportTreeshakenRE.lastIndex - match[6]?.length
] = { declaration: `const {${names}}`, names: `{ ${names} }` }
sun0day marked this conversation as resolved.
Show resolved Hide resolved
}
}

let s: MagicString | undefined
const str = () => s || (s = new MagicString(source))
let needPreloadHelper = false
Expand Down Expand Up @@ -265,7 +328,26 @@ export function buildImportAnalysisPlugin(config: ResolvedConfig): Plugin {
source[start] === '`')
) {
needPreloadHelper = true
str().prependLeft(expStart, `${preloadMethod}(() => `)
const { declaration, names } = dynamicImports[expEnd] || {}
if (names) {
/* transform `const {foo} = await import('foo')`
* to `const {foo} = await __vitePreload(async () => { const {foo} = await import('foo');return {foo}}, ...)`
*
* transform `import('foo').then(({foo})=>{})`
* to `__vitePreload(async () => { const {foo} = await import('foo');return { foo }},...).then(({foo})=>{})`
*
* transform `(await import('foo')).foo`
* to `__vitePreload(async () => { const {foo} = (await import('foo')).foo; return { foo }},...)).foo`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upgrading to vite 5.3.0-beta.1 has broken a storybook build, and I wonder if it's because of this PR. Is it possible that this can cause an invalid shorthand object? In my case, I have let axe=(await import('axe-core')).default, and I'm getting an error during build: RollupError: Cannot use a reserved word as a shorthand property

Is it possible that this is being transformed to {default}?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so! We may need to handle this like on line 278. Seems like we need more tests that covers default

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@IanVS FWIW, hand editing the storybook code to let {default: x} = (await import('axe-core')) fixes the problem, so your theory seems confirmed.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI I've released a workaround in Storybook https://github.com/storybookjs/storybook/releases/tag/v8.1.9

*/
str().prependLeft(
expStart,
`${preloadMethod}(async () => { ${declaration} = await `,
)
str().appendRight(expEnd, `;return ${names}}`)
} else {
str().prependLeft(expStart, `${preloadMethod}(() => `)
}

str().appendRight(
expEnd,
`,${isModernFlag}?${preloadMarker}:void 0${
Expand Down
19 changes: 19 additions & 0 deletions playground/dynamic-import/__tests__/dynamic-import.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { expect, test } from 'vitest'
import {
browserLogs,
findAssetFile,
getColor,
isBuild,
Expand Down Expand Up @@ -178,6 +179,24 @@ test.runIf(isBuild)(
},
)

test('dynamic import treeshaken log', async () => {
const log = browserLogs.join('\n')
expect(log).toContain('treeshaken foo')
expect(log).toContain('treeshaken bar')
expect(log).toContain('treeshaken baz1')
expect(log).toContain('treeshaken baz2')
expect(log).toContain('treeshaken baz3')
expect(log).toContain('treeshaken baz4')
expect(log).toContain('treeshaken baz5')
expect(log).toContain('treeshaken default')

expect(log).not.toContain('treeshaken removed')
})

test.runIf(isBuild)('dynamic import treeshaken file', async () => {
expect(findAssetFile(/treeshaken.+\.js$/)).not.toContain('treeshaken removed')
})

test.runIf(isBuild)('should not preload for non-analyzable urls', () => {
const js = findAssetFile(/index-[-\w]{8}\.js$/)
// should match e.g. await import(e.jss);o(".view",p===i)
Expand Down
23 changes: 23 additions & 0 deletions playground/dynamic-import/nested/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,29 @@ import(`../nested/${base}.js`).then((mod) => {
import(`../nested/nested/${base}.js`).then((mod) => {
text('.dynamic-import-nested-self', mod.self)
})
;(async function () {
const { foo } = await import('./treeshaken/treeshaken.js')
const { bar, default: tree } = await import('./treeshaken/treeshaken.js')
const baz1 = (await import('./treeshaken/treeshaken.js')).baz1
const baz2 = (await import('./treeshaken/treeshaken.js')).baz2.log
const baz3 = (await import('./treeshaken/treeshaken.js')).baz3?.log
const baz4 = await import('./treeshaken/treeshaken.js').then(
({ baz4 }) => baz4,
)
const baz5 = await import('./treeshaken/treeshaken.js').then(function ({
baz5,
}) {
return baz5
})
foo()
bar()
tree()
baz1()
baz2()
baz3()
baz4()
baz5()
})()

import(`../nested/static.js`).then((mod) => {
text('.dynamic-import-static', mod.self)
Expand Down
31 changes: 31 additions & 0 deletions playground/dynamic-import/nested/treeshaken/treeshaken.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
export const foo = () => {
console.log('treeshaken foo')
}
export const bar = () => {
console.log('treeshaken bar')
}
export const baz1 = () => {
console.log('treeshaken baz1')
}
export const baz2 = {
log: () => {
console.log('treeshaken baz2')
},
}
export const baz3 = {
log: () => {
console.log('treeshaken baz3')
},
}
export const baz4 = () => {
console.log('treeshaken baz4')
}
export const baz5 = () => {
console.log('treeshaken baz5')
}
export const removed = () => {
console.log('treeshaken removed')
}
export default () => {
console.log('treeshaken default')
}