Skip to content

Commit

Permalink
fix(optimizer): external require-import conversion (fixes #2492, #3409)…
Browse files Browse the repository at this point in the history
… (#8459)
  • Loading branch information
sapphi-red authored Jun 5, 2022
1 parent 129b499 commit 1061bbd
Show file tree
Hide file tree
Showing 9 changed files with 65 additions and 3 deletions.
28 changes: 27 additions & 1 deletion packages/vite/src/node/optimizer/esbuildDepPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ import {
import { browserExternalId } from '../plugins/resolve'
import type { ExportsData } from '.'

const externalWithConversionNamespace =
'vite:dep-pre-bundle:external-conversion'
const convertedExternalPrefix = 'vite-dep-pre-bundle-external:'

const externalTypes = [
'css',
// supported pre-processor types
Expand Down Expand Up @@ -80,20 +84,42 @@ export function esbuildDepPlugin(
name: 'vite:dep-pre-bundle',
setup(build) {
// externalize assets and commonly known non-js file types
// See #8459 for more details about this require-import conversion
build.onResolve(
{
filter: new RegExp(`\\.(` + allExternalTypes.join('|') + `)(\\?.*)?$`)
},
async ({ path: id, importer, kind }) => {
// if the prefix exist, it is already converted to `import`, so set `external: true`
if (id.startsWith(convertedExternalPrefix)) {
return {
path: id.slice(convertedExternalPrefix.length),
external: true
}
}

const resolved = await resolve(id, importer, kind)
if (resolved) {
// here it is not set to `external: true` to convert `require` to `import`
return {
path: resolved,
external: true
namespace: externalWithConversionNamespace
}
}
}
)
build.onLoad(
{ filter: /./, namespace: externalWithConversionNamespace },
(args) => {
// import itself with prefix (this is the actual part of require-import conversion)
return {
contents:
`export { default } from "${convertedExternalPrefix}${args.path}";` +
`export * from "${convertedExternalPrefix}${args.path}";`,
loader: 'js'
}
}
)

function resolveEntry(id: string) {
const flatId = flattenId(id)
Expand Down
6 changes: 5 additions & 1 deletion playground/optimize-deps/__tests__/optimize-deps.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,11 @@ test('dep with dynamic import', async () => {
})

test('dep with css import', async () => {
expect(await getColor('h1')).toBe('red')
expect(await getColor('.dep-linked-include')).toBe('red')
})

test('CJS dep with css import', async () => {
expect(await getColor('.cjs-with-assets')).toBe('blue')
})

test('dep w/ non-js files handled via plugin', async () => {
Expand Down
3 changes: 3 additions & 0 deletions playground/optimize-deps/dep-cjs-with-assets/foo.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.cjs-with-assets {
color: blue;
}
3 changes: 3 additions & 0 deletions playground/optimize-deps/dep-cjs-with-assets/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
require('./foo.css')

exports.a = 11
6 changes: 6 additions & 0 deletions playground/optimize-deps/dep-cjs-with-assets/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "dep-cjs-with-assets",
"private": true,
"version": "0.0.0",
"main": "index.js"
}
2 changes: 1 addition & 1 deletion playground/optimize-deps/dep-linked-include/test.css
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
body {
.dep-linked-include {
color: red;
}
8 changes: 8 additions & 0 deletions playground/optimize-deps/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ <h2>Detecting linked src package and optimizing its deps (lodash-es)</h2>
<h2>Optimizing force included dep even when it's linked</h2>
<div class="force-include"></div>

<h2>Dep with CSS</h2>
<div class="dep-linked-include">This should be red</div>

<h2>CJS Dep with CSS</h2>
<div class="cjs-with-assets">This should be blue</div>

<h2>import * as ...</h2>
<div class="import-star"></div>

Expand Down Expand Up @@ -91,6 +97,8 @@ <h2>Reused variable names</h2>
text('.import-star', `[success] ${keys.join(', ')}`)
}

import 'dep-cjs-with-assets'

import { env } from 'dep-node-env'
text('.node-env', env)

Expand Down
1 change: 1 addition & 0 deletions playground/optimize-deps/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
"clipboard": "^2.0.11",
"dep-cjs-compiled-from-cjs": "file:./dep-cjs-compiled-from-cjs",
"dep-cjs-compiled-from-esm": "file:./dep-cjs-compiled-from-esm",
"dep-cjs-with-assets": "file:./dep-cjs-with-assets",
"dep-esbuild-plugin-transform": "file:./dep-esbuild-plugin-transform",
"dep-linked": "link:./dep-linked",
"dep-linked-include": "link:./dep-linked-include",
Expand Down
11 changes: 11 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 1061bbd

Please sign in to comment.