Skip to content

Commit

Permalink
fix: allow reusing variable names between scripts
Browse files Browse the repository at this point in the history
  • Loading branch information
@OliverTsang
OliverTsang committed Feb 8, 2022
1 parent b79fec9 commit 661a358
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 43 deletions.
4 changes: 4 additions & 0 deletions packages/playground/optimize-deps/__tests__/optimize-deps.spec.ts
View file
Expand Up @@ -87,3 +87,7 @@ test('import from hidden dir', async () => {
test('import optimize-excluded package that imports optimized-included package', async () => {
expect(await page.textContent('.nested-include')).toBe('nested-include')
})

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
View file
Expand Up @@ -53,6 +53,15 @@ <h2>Dep from hidden dir</h2>
<h2>Nested include</h2>
<div>Module path: <span class="nested-include"></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 @@ -84,8 +93,12 @@ <h2>Nested include</h2>

import { nestedInclude } from 'nested-exclude'
text('.nested-include', nestedInclude)
</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>
73 changes: 33 additions & 40 deletions packages/vite/src/node/optimizer/scan.ts
View file
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 @@ -149,7 +147,6 @@ export const commentRE = /<!--(.|[\r\n])*?-->/
const srcRE = /\bsrc\s*=\s*(?:"([^"]+)"|'([^']+)'|([^\s'">]+))/im
const typeRE = /\btype\s*=\s*(?:"([^"]+)"|'([^']+)'|([^\s'">]+))/im
const langRE = /\blang\s*=\s*(?:"([^"]+)"|'([^']+)'|([^\s'">]+))/im
const contextRE = /\bcontext\s*=\s*(?:"([^"]+)"|'([^']+)'|([^\s'">]+))/im

function esbuildScanPlugin(
config: ResolvedConfig,
Expand Down Expand Up @@ -189,7 +186,8 @@ function esbuildScanPlugin(
return {
name: 'vite:dep-scan',
setup(build) {
const localScripts: Record<string, OnLoadResult> = {}
const scripts: Record<string, OnLoadResult> = {}
let scriptId = 0

// external urls
build.onResolve({ filter: externalRE }, ({ path }) => ({
Expand All @@ -208,12 +206,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 @@ -264,37 +262,40 @@ 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
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

// 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
)
}
js += `import '${virtualModulePrefix}${path}';\n`
} else {
js += content + '\n'
scripts[key] = {
loader,
contents
}
}
}
}

// `<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)
js += `import '${virtualModulePrefix}${key}';\n`
}
}

// This will trigger incorrectly if `export default` is contained
Expand All @@ -305,14 +306,6 @@ 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)
}
}

return {
loader,
contents: js
Expand Down

0 comments on commit 661a358

Please sign in to comment.