Skip to content

Commit

Permalink
fix(optimize-deps): don't externalize JS files imported with asset ex…
Browse files Browse the repository at this point in the history
…tensions (#16242)
  • Loading branch information
proc07 authored May 30, 2024
1 parent e6a70b7 commit 4161843
Show file tree
Hide file tree
Showing 12 changed files with 86 additions and 0 deletions.
8 changes: 8 additions & 0 deletions packages/vite/src/node/optimizer/esbuildDepPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,14 @@ export function esbuildDepPlugin(
const resolved = await resolve(id, importer, kind)
if (resolved) {
if (kind === 'require-call') {
// #16116 fix: Import the module.scss path, which is actually module.scss.js
if (resolved.endsWith('.js')) {
return {
path: resolved,
external: false,
}
}

// here it is not set to `external: true` to convert `require` to `import`
return {
path: resolved,
Expand Down
15 changes: 15 additions & 0 deletions playground/optimize-deps/__tests__/optimize-deps.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -331,3 +331,18 @@ test.runIf(isServe)('warn on incompatible dependency', () => {
),
)
})

test('import the CommonJS external package that omits the js suffix', async () => {
await expectWithRetry(() => page.textContent('.external-package-js')).toBe(
'okay',
)
await expectWithRetry(() =>
page.textContent('.external-package-scss-js'),
).toBe('scss')
await expectWithRetry(() =>
page.textContent('.external-package-astro-js'),
).toBe('astro')
await expectWithRetry(() =>
page.textContent('.external-package-tsx-js'),
).toBe('tsx')
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
const { okay } = require('./test.okay')
const { scss } = require('./test.scss')
const { astro } = require('./test.astro')
const { tsx } = require('./test.tsx')

module.exports = { okay, scss, astro, tsx }
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "@vitejs/test-dep-cjs-external-package-omit-js-suffix",
"private": true,
"version": "0.0.0",
"main": "index.js"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
function astro() {
return 'astro'
}

module.exports = { astro }
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
function okay() {
return 'okay'
}

module.exports = { okay }
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
function scss() {
return 'scss'
}

module.exports = { scss }
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
function tsx() {
return 'tsx'
}

module.exports = { tsx }
19 changes: 19 additions & 0 deletions playground/optimize-deps/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,25 @@ <h2>Long file name import works</h2>

<script type="module" src="./long-file-name.js"></script>

<h2>Import the CommonJS external package that omits the js suffix</h2>
<div class="external-package-js"></div>
<div class="external-package-scss-js"></div>
<div class="external-package-astro-js"></div>
<div class="external-package-tsx-js"></div>
<script type="module">
import {
astro,
okay,
scss,
tsx,
} from '@vitejs/test-dep-cjs-external-package-omit-js-suffix'

text('.external-package-js', okay())
text('.external-package-scss-js', scss())
text('.external-package-astro-js', astro())
text('.external-package-tsx-js', tsx())
</script>

<script>
function text(el, text) {
document.querySelector(el).textContent = text
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 @@ -36,6 +36,7 @@
"@vitejs/test-dep-with-optional-peer-dep-submodule": "file:./dep-with-optional-peer-dep-submodule",
"@vitejs/test-dep-non-optimized": "file:./dep-non-optimized",
"@vitejs/test-added-in-entries": "file:./added-in-entries",
"@vitejs/test-dep-cjs-external-package-omit-js-suffix": "file:./dep-cjs-external-package-omit-js-suffix",
"lodash-es": "^4.17.21",
"@vitejs/test-nested-exclude": "file:./nested-exclude",
"phoenix": "^1.7.12",
Expand Down
1 change: 1 addition & 0 deletions playground/optimize-deps/vite.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export default defineConfig({
include: [
'@vitejs/test-dep-linked-include',
'@vitejs/test-nested-exclude > @vitejs/test-nested-include',
'@vitejs/test-dep-cjs-external-package-omit-js-suffix',
// will throw if optimized (should log warning instead)
'@vitejs/test-non-optimizable-include',
'@vitejs/test-dep-optimize-exports-with-glob/**/*',
Expand Down
10 changes: 10 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 4161843

Please sign in to comment.