Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
fix: reusing variable names in html module scripts (fix #6851) (#6818)
  • Loading branch information
OliverTsang committed Mar 4, 2022
1 parent 79dd003 commit c46b56d
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 47 deletions.
Expand Up @@ -95,3 +95,7 @@ test('import optimize-excluded package that imports optimized-included package',
test('import aliased package with colon', async () => {
expect(await page.textContent('.url')).toBe('vitejs.dev')
})

test('variable names are reused in different scripts', async () => {
expect(await page.textContent('.reused-variable-names')).toBe('reused')
})
19 changes: 16 additions & 3 deletions packages/playground/optimize-deps/index.html
Expand Up @@ -59,6 +59,15 @@ <h2>Nested include</h2>
<h2>Alias with colon</h2>
<div>URL: <span class="url"></span></div>

<h2>Reused variable names</h2>
<div>This should show reused: <span class="reused-variable-names"></span></div>

<script>
function text(el, text) {
document.querySelector(el).textContent = text
}
</script>

<script type="module">
// test dep detection in globbed files
const globbed = import.meta.globEager('./glob/*.js')
Expand Down Expand Up @@ -96,8 +105,12 @@ <h2>Alias with colon</h2>

import { parse } from 'node:url'
text('.url', parse('https://vitejs.dev').hostname)
</script>

function text(el, text) {
document.querySelector(el).textContent = text
}
<script type="module">
const reusedName = 1
</script>
<script type="module">
const reusedName = 'reused'
text('.reused-variable-names', reusedName)
</script>
93 changes: 49 additions & 44 deletions packages/vite/src/node/optimizer/scan.ts
Expand Up @@ -35,8 +35,6 @@ const debug = createDebugger('vite:deps')

const htmlTypesRE = /\.(html|vue|svelte|astro)$/

const setupRE = /<script\s+setup/

// A simple regex to detect import sources. This is only used on
// <script lang="ts"> blocks in vue (setup only) or svelte files, since
// seemingly unused imports are dropped by esbuild when transpiling TS which
Expand Down Expand Up @@ -193,7 +191,7 @@ function esbuildScanPlugin(
return {
name: 'vite:dep-scan',
setup(build) {
const localScripts: Record<string, OnLoadResult> = {}
const scripts: Record<string, OnLoadResult> = {}

// external urls
build.onResolve({ filter: externalRE }, ({ path }) => ({
Expand All @@ -212,12 +210,12 @@ function esbuildScanPlugin(
return {
// strip prefix to get valid filesystem path so esbuild can resolve imports in the file
path: path.replace(virtualModulePrefix, ''),
namespace: 'local-script'
namespace: 'script'
}
})

build.onLoad({ filter: /.*/, namespace: 'local-script' }, ({ path }) => {
return localScripts[path]
build.onLoad({ filter: /.*/, namespace: 'script' }, ({ path }) => {
return scripts[path]
})

// html types: extract script contents -----------------------------------
Expand Down Expand Up @@ -245,7 +243,7 @@ function esbuildScanPlugin(
const regex = isHtml ? scriptModuleRE : scriptRE
regex.lastIndex = 0
let js = ''
let loader: Loader = 'js'
let scriptId = 0
let match: RegExpExecArray | null
while ((match = regex.exec(raw))) {
const [, openTag, content] = match
Expand All @@ -266,6 +264,7 @@ function esbuildScanPlugin(
) {
continue
}
let loader: Loader = 'js'
if (lang === 'ts' || lang === 'tsx' || lang === 'jsx') {
loader = lang
}
Expand All @@ -274,39 +273,59 @@ function esbuildScanPlugin(
const src = srcMatch[1] || srcMatch[2] || srcMatch[3]
js += `import ${JSON.stringify(src)}\n`
} else if (content.trim()) {
// There can be module scripts (`<script context="module">` in Svelte and `<script>` in Vue)
// The reason why virtual modules are needed:
// 1. There can be module scripts (`<script context="module">` in Svelte and `<script>` in Vue)
// or local scripts (`<script>` in Svelte and `<script setup>` in Vue)
// 2. There can be multiple module scripts in html
// We need to handle these separately in case variable names are reused between them

// append imports in TS to prevent esbuild from removing them
// since they may be used in the template
const contents =
content +
(loader.startsWith('ts') ? extractImportPaths(content) : '')

const key = `${path}?id=${scriptId++}`

if (contents.includes('import.meta.glob')) {
scripts[key] = {
// transformGlob already transforms to js
loader: 'js',
contents: await transformGlob(
contents,
path,
config.root,
loader,
resolve
)
}
} else {
scripts[key] = {
loader,
contents
}
}

const virtualModulePath = JSON.stringify(
virtualModulePrefix + key
)

const contextMatch = openTag.match(contextRE)
const context =
contextMatch &&
(contextMatch[1] || contextMatch[2] || contextMatch[3])
if (
(path.endsWith('.vue') && setupRE.test(openTag)) ||
(path.endsWith('.svelte') && context !== 'module')
) {
// append imports in TS to prevent esbuild from removing them
// since they may be used in the template
const localContent =
content +
(loader.startsWith('ts') ? extractImportPaths(content) : '')
localScripts[path] = {
loader,
contents: localContent
}
js += `import ${JSON.stringify(virtualModulePrefix + path)}\n`

// Especially for Svelte files, exports in <script context="module"> means module exports,
// exports in <script> means component props. To avoid having two same export name from the
// star exports, we need to ignore exports in <script>
if (path.endsWith('.svelte') && context !== 'module') {
js += `import ${virtualModulePath}\n`
} else {
js += content + '\n'
js += `export * from ${virtualModulePath}\n`
}
}
}

// `<script>` in Svelte has imports that can be used in the template
// so we handle them here too
if (loader.startsWith('ts') && path.endsWith('.svelte')) {
js += extractImportPaths(js)
}

// This will trigger incorrectly if `export default` is contained
// anywhere in a string. Svelte and Astro files can't have
// `export default` as code so we know if it's encountered it's a
Expand All @@ -315,22 +334,8 @@ function esbuildScanPlugin(
js += '\nexport default {}'
}

if (js.includes('import.meta.glob')) {
return {
// transformGlob already transforms to js
loader: 'js',
contents: await transformGlob(
js,
path,
config.root,
loader,
resolve
)
}
}

return {
loader,
loader: 'js',
contents: js
}
}
Expand Down

0 comments on commit c46b56d

Please sign in to comment.