Skip to content

Commit

Permalink
fix: preserve default export from externalized packages (fixes #10258) (
Browse files Browse the repository at this point in the history
#10406)

fixes #10258
  • Loading branch information
sapphi-red committed Nov 24, 2022
1 parent d6b4ee5 commit 88b001b
Show file tree
Hide file tree
Showing 15 changed files with 119 additions and 55 deletions.
42 changes: 31 additions & 11 deletions packages/vite/src/node/optimizer/esbuildDepPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ const externalWithConversionNamespace =
'vite:dep-pre-bundle:external-conversion'
const convertedExternalPrefix = 'vite-dep-pre-bundle-external:'

const cjsExternalFacadeNamespace = 'vite:cjs-external-facade'
const nonFacadePrefix = 'vite-cjs-external-facade:'

const externalTypes = [
'css',
// supported pre-processor types
Expand Down Expand Up @@ -268,19 +271,36 @@ export function esbuildCjsExternalPlugin(externals: string[]): Plugin {
`^${text.replace(/[-/\\^$*+?.()|[\]{}]/g, '\\$&')}$`
const filter = new RegExp(externals.map(escape).join('|'))

build.onResolve({ filter: /.*/, namespace: 'external' }, (args) => ({
path: args.path,
external: true
}))
build.onResolve({ filter: new RegExp(`^${nonFacadePrefix}`) }, (args) => {
return {
path: args.path.slice(nonFacadePrefix.length),
external: true
}
})

build.onResolve({ filter }, (args) => {
if (args.kind === 'require-call') {
return {
path: args.path,
namespace: cjsExternalFacadeNamespace
}
}

build.onResolve({ filter }, (args) => ({
path: args.path,
namespace: 'external'
}))
return {
path: args.path,
external: true
}
})

build.onLoad({ filter: /.*/, namespace: 'external' }, (args) => ({
contents: `export * from ${JSON.stringify(args.path)}`
}))
build.onLoad(
{ filter: /.*/, namespace: cjsExternalFacadeNamespace },
(args) => ({
contents:
`import * as m from ${JSON.stringify(
nonFacadePrefix + args.path
)};` + `module.exports = m;`
})
)
}
}
}
6 changes: 6 additions & 0 deletions playground/external/__tests__/external.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ test('importmap', () => {
)
})

test('should have default exports', async () => {
expect(await page.textContent('#imported-slash5-exists')).toBe('true')
expect(await page.textContent('#imported-slash3-exists')).toBe('true')
expect(await page.textContent('#required-slash3-exists')).toBe('true')
})

describe.runIf(isBuild)('build', () => {
test('should externalize imported packages', async () => {
// If `vue` is successfully externalized, the page should use the version from the import map
Expand Down
3 changes: 0 additions & 3 deletions playground/external/dep-that-imports-vue/index.js

This file was deleted.

8 changes: 0 additions & 8 deletions playground/external/dep-that-imports-vue/package.json

This file was deleted.

9 changes: 9 additions & 0 deletions playground/external/dep-that-imports/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { version } from 'vue'
import slash5 from 'slash5'
import slash3 from 'slash3'

document.querySelector('#imported-vue-version').textContent = version
document.querySelector('#imported-slash5-exists').textContent =
!!slash5('foo/bar')
document.querySelector('#imported-slash3-exists').textContent =
!!slash3('foo/bar')
10 changes: 10 additions & 0 deletions playground/external/dep-that-imports/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"name": "@vitejs/dep-that-imports",
"private": true,
"version": "0.0.0",
"dependencies": {
"slash3": "npm:slash@^3.0.0",
"slash5": "npm:slash@^5.0.0",
"vue": "^3.2.45"
}
}
3 changes: 0 additions & 3 deletions playground/external/dep-that-requires-vue/index.js

This file was deleted.

8 changes: 0 additions & 8 deletions playground/external/dep-that-requires-vue/package.json

This file was deleted.

7 changes: 7 additions & 0 deletions playground/external/dep-that-requires/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
const { version } = require('vue')
// require('slash5') // cannot require ESM
const slash3 = require('slash3')

document.querySelector('#required-vue-version').textContent = version
document.querySelector('#required-slash3-exists').textContent =
!!slash3('foo/bar')
10 changes: 10 additions & 0 deletions playground/external/dep-that-requires/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"name": "@vitejs/dep-that-requires",
"private": true,
"version": "0.0.0",
"dependencies": {
"slash3": "npm:slash@^3.0.0",
"slash5": "npm:slash@^5.0.0",
"vue": "^3.2.45"
}
}
7 changes: 6 additions & 1 deletion playground/external/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,19 @@
<script type="importmap">
{
"imports": {
"vue": "https://unpkg.com/vue@3.2.0/dist/vue.runtime.esm-browser.js"
"vue": "https://unpkg.com/vue@3.2.0/dist/vue.runtime.esm-browser.js",
"slash5": "https://unpkg.com/slash@5.0.0/index.js",
"slash3": "https://esm.sh/slash@3.0.0"
}
}
</script>
</head>
<body>
<p>Imported Vue version: <span id="imported-vue-version"></span></p>
<p>Required Vue version: <span id="required-vue-version"></span></p>
<p>Imported slash5 exists: <span id="imported-slash5-exists"></span></p>
<p>Imported slash3 exists: <span id="imported-slash3-exists"></span></p>
<p>Required slash3 exists: <span id="required-slash3-exists"></span></p>
<script type="module" src="/src/main.js"></script>
</body>
</html>
6 changes: 4 additions & 2 deletions playground/external/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@
"preview": "vite preview"
},
"dependencies": {
"@vitejs/dep-that-imports-vue": "file:./dep-that-imports-vue",
"@vitejs/dep-that-requires-vue": "file:./dep-that-requires-vue"
"@vitejs/dep-that-imports": "file:./dep-that-imports",
"@vitejs/dep-that-requires": "file:./dep-that-requires"
},
"devDependencies": {
"slash3": "npm:slash@^3.0.0",
"slash5": "npm:slash@^5.0.0",
"vite": "workspace:*",
"vue": "^3.2.45"
}
Expand Down
4 changes: 2 additions & 2 deletions playground/external/src/main.js
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
import '@vitejs/dep-that-imports-vue'
import '@vitejs/dep-that-requires-vue'
import '@vitejs/dep-that-imports'
import '@vitejs/dep-that-requires'
8 changes: 6 additions & 2 deletions playground/external/vite.config.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
import { defineConfig } from 'vite'

export default defineConfig({
optimizeDeps: {
include: ['dep-that-imports', 'dep-that-requires'],
exclude: ['vue', 'slash5']
},
build: {
minify: false,
rollupOptions: {
external: ['vue']
external: ['vue', 'slash3', 'slash5']
},
commonjsOptions: {
esmExternals: ['vue']
esmExternals: ['vue', 'slash5']
}
}
})
43 changes: 28 additions & 15 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 88b001b

Please sign in to comment.