Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(optimizer): nested optimization #4634

Merged
merged 16 commits into from Aug 31, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 10 additions & 1 deletion docs/config/index.md
Expand Up @@ -713,7 +713,16 @@ createServer()
Dependencies to exclude from pre-bundling.

:::warning CommonJS
CommonJS dependencies should not be excluded from optimization. If an ESM dependency has a nested CommonJS dependency, it should not be excluded as well.
CommonJS dependencies should not be excluded from optimization. If an ESM dependency is excluded from optimization, but has a nested CommonJS dependency, the CommonJS dependency should be added to `optimizeDeps.include`. Example:

```js
export default defineConfig({
optimizeDeps: {
include: ['esm-dep > cjs-dep']
}
})
```

:::

### optimizeDeps.include
Expand Down
4 changes: 2 additions & 2 deletions packages/playground/dynamic-import/css/index.css
@@ -1,2 +1,2 @@
.css { box-sizing: border-box; }
.view { color: red; }
.css { box-sizing: border-box; }
.view { color: red; }
2 changes: 2 additions & 0 deletions packages/playground/nested-deps/__tests__/nested-deps.spec.ts
Expand Up @@ -5,4 +5,6 @@ test('handle nested package', async () => {
const c = await page.textContent('.c')
expect(c).toBe('es-C@1.0.0')
expect(await page.textContent('.side-c')).toBe(c)
expect(await page.textContent('.d')).toBe('D@1.0.0')
expect(await page.textContent('.nested-d')).toBe('D-nested@1.0.0')
})
10 changes: 10 additions & 0 deletions packages/playground/nested-deps/index.html
Expand Up @@ -13,11 +13,18 @@ <h2>direct dependency C</h2>
<h2>side dependency C</h2>
<pre class="side-c"></pre>

<h2>direct dependency D</h2>
<pre class="d"></pre>

<h2>nested dependency nested-D (dep of D)</h2>
<pre class="nested-d"></pre>

<script type="module">
import A from 'test-package-a'
import B, { A as nestedA } from 'test-package-b'
import C from 'test-package-c'
import { C as sideC } from 'test-package-c/side'
import D, { nestedD } from 'test-package-d'

text('.a', A)
text('.b', B)
Expand All @@ -26,6 +33,9 @@ <h2>side dependency C</h2>
text('.c', C)
text('.side-c', sideC)

text('.d', D)
text('.nested-d', nestedD)

function text(sel, text) {
document.querySelector(sel).textContent = text
}
Expand Down
3 changes: 2 additions & 1 deletion packages/playground/nested-deps/package.json
Expand Up @@ -11,6 +11,7 @@
"dependencies": {
"test-package-a": "link:./test-package-a",
"test-package-b": "link:./test-package-b",
"test-package-c": "link:./test-package-c"
"test-package-c": "link:./test-package-c",
"test-package-d": "link:./test-package-d"
}
}
3 changes: 3 additions & 0 deletions packages/playground/nested-deps/test-package-d/index.js
@@ -0,0 +1,3 @@
export { default as nestedD } from 'test-package-d-nested'

export default 'D@1.0.0'
8 changes: 8 additions & 0 deletions packages/playground/nested-deps/test-package-d/package.json
@@ -0,0 +1,8 @@
{
"name": "test-package-d",
"version": "1.0.0",
"main": "index.js",
"dependencies": {
"test-package-d-nested": "link:./test-package-d-nested"
}
}
@@ -0,0 +1 @@
export default 'D-nested@1.0.0'
@@ -0,0 +1,5 @@
{
"name": "test-package-d-nested",
"version": "1.0.0",
"main": "index.js"
}
6 changes: 4 additions & 2 deletions packages/playground/nested-deps/vite.config.js
Expand Up @@ -7,7 +7,9 @@ module.exports = {
'test-package-a',
'test-package-b',
'test-package-c',
'test-package-c/side'
]
'test-package-c/side',
'test-package-d > test-package-d-nested'
],
exclude: ['test-package-d']
}
}
Expand Up @@ -83,3 +83,7 @@ test('esbuild-plugin', async () => {
test('import from hidden dir', async () => {
expect(await page.textContent('.hidden-dir')).toBe('hello!')
})

test('import optimize-excluded package that imports optimized-included package', async () => {
expect(await page.textContent('.nested-include')).toBe('nested-include')
})
6 changes: 6 additions & 0 deletions packages/playground/optimize-deps/index.html
Expand Up @@ -50,6 +50,9 @@ <h2>Dep with changes from esbuild plugin</h2>
<h2>Dep from hidden dir</h2>
<div>This should show hello!: <span class="hidden-dir"></span></div>

<h2>Nested include</h2>
<div>Module path: <span class="nested-include"></span></div>

<script type="module">
// test dep detection in globbed files
const globbed = import.meta.globEager('./glob/*.js')
Expand Down Expand Up @@ -79,6 +82,9 @@ <h2>Dep from hidden dir</h2>
import { greeting } from './.hidden-dir/foo.js'
text('.hidden-dir', greeting)

import { nestedInclude } from 'nested-exclude'
text('.nested-include', nestedInclude)

function text(el, text) {
document.querySelector(el).textContent = text
}
Expand Down
3 changes: 3 additions & 0 deletions packages/playground/optimize-deps/nested-exclude/index.js
@@ -0,0 +1,3 @@
export { default as nestedInclude } from 'nested-include'

export default 'nested-exclude'
@@ -0,0 +1,2 @@
// written in cjs, optimization should convert this to esm
module.exports = 'nested-include'
@@ -0,0 +1,5 @@
{
"name": "nested-include",
"version": "1.0.0",
"main": "index.js"
}
8 changes: 8 additions & 0 deletions packages/playground/optimize-deps/nested-exclude/package.json
@@ -0,0 +1,8 @@
{
"name": "nested-exclude",
"version": "1.0.0",
"main": "index.js",
"dependencies": {
"nested-include": "file:./nested-include"
}
}
1 change: 1 addition & 0 deletions packages/playground/optimize-deps/package.json
Expand Up @@ -16,6 +16,7 @@
"dep-esbuild-plugin-transform": "link:./dep-esbuild-plugin-transform",
"dep-cjs-compiled-from-esm": "file:./dep-cjs-compiled-from-esm",
"dep-cjs-compiled-from-cjs": "file:./dep-cjs-compiled-from-cjs",
"nested-exclude": "link:./nested-exclude",
"phoenix": "^1.5.7",
"react": "^17.0.1",
"react-dom": "^17.0.1",
Expand Down
4 changes: 3 additions & 1 deletion packages/playground/optimize-deps/vite.config.js
Expand Up @@ -12,8 +12,10 @@ module.exports = {
include: [
'dep-linked-include',
// required since it isn't in node_modules and is ignored by the optimizer otherwise
'dep-esbuild-plugin-transform'
'dep-esbuild-plugin-transform',
'nested-exclude>nested-include'
],
exclude: ['nested-exclude'],
esbuildOptions: {
plugins: [
{
Expand Down
10 changes: 7 additions & 3 deletions packages/vite/src/node/optimizer/index.ts
Expand Up @@ -10,7 +10,8 @@ import {
lookupFile,
normalizePath,
writeFile,
flattenId
flattenId,
normalizeId
} from '../utils'
import { esbuildDepPlugin } from './esbuildDepPlugin'
import { ImportSpecifier, init, parse } from 'es-module-lexer'
Expand Down Expand Up @@ -184,10 +185,13 @@ export async function optimizeDeps(
if (include) {
const resolve = config.createResolver({ asSrc: false })
for (const id of include) {
if (!deps[id]) {
// normalize 'foo >bar` as 'foo > bar' to prevent same id being added
// and for pretty printing
const normalizedId = normalizeId(id)
if (!deps[normalizedId]) {
const entry = await resolve(id)
if (entry) {
deps[id] = entry
deps[normalizedId] = entry
} else {
throw new Error(
`Failed to resolve force included dependency: ${chalk.cyan(id)}`
Expand Down
2 changes: 1 addition & 1 deletion packages/vite/src/node/plugins/clientInjections.ts
Expand Up @@ -30,7 +30,7 @@ export function clientInjectionsPlugin(config: ResolvedConfig): Plugin {
if (config.server.middlewareMode) {
port = String(port || 24678)
} else {
port = String(port ||options.port || config.server.port!)
port = String(port || options.port || config.server.port!)
}
let hmrBase = config.base
if (options.path) {
Expand Down
4 changes: 2 additions & 2 deletions packages/vite/src/node/plugins/preAlias.ts
Expand Up @@ -13,9 +13,9 @@ export function preAliasPlugin(): Plugin {
configureServer(_server) {
server = _server
},
resolveId(id, _, __, ssr) {
resolveId(id, importer, _, ssr) {
if (!ssr && bareImportRE.test(id)) {
return tryOptimizedResolve(id, server)
return tryOptimizedResolve(id, server, importer)
}
}
}
Expand Down
80 changes: 66 additions & 14 deletions packages/vite/src/node/plugins/resolve.ts
Expand Up @@ -23,7 +23,8 @@ import {
resolveFrom,
isDataUrl,
cleanUrl,
slash
slash,
nestedResolveFrom
} from '../utils'
import { ViteDevServer, SSROptions } from '..'
import { createFilter } from '@rollup/pluginutils'
Expand Down Expand Up @@ -204,7 +205,7 @@ export function resolvePlugin(baseOptions: InternalResolveOptions): Plugin {
asSrc &&
server &&
!ssr &&
(res = tryOptimizedResolve(id, server))
(res = tryOptimizedResolve(id, server, importer))
) {
return res
}
Expand Down Expand Up @@ -389,8 +390,18 @@ export function tryNodeResolve(
ssr?: boolean
): PartialResolvedId | undefined {
const { root, dedupe, isBuild } = options
const deepMatch = id.match(deepImportRE)
const pkgId = deepMatch ? deepMatch[1] || deepMatch[2] : id

// split id by last '>' for nested selected packages, for example:
// 'foo > bar > baz' => 'foo > bar' & 'baz'
// 'foo' => '' & 'foo'
const lastArrowIndex = id.lastIndexOf('>')
const nestedRoot = id.substring(0, lastArrowIndex).trim()
const nestedPath = id.substring(lastArrowIndex + 1).trim()

// check for deep import, e.g. "my-lib/foo"
const deepMatch = nestedPath.match(deepImportRE)

const pkgId = deepMatch ? deepMatch[1] || deepMatch[2] : nestedPath

let basedir: string
if (dedupe && dedupe.includes(pkgId)) {
Expand All @@ -405,6 +416,11 @@ export function tryNodeResolve(
basedir = root
}

// nested node module, step-by-step resolve to the basedir of the nestedPath
if (nestedRoot) {
basedir = nestedResolveFrom(nestedRoot, basedir)
}

const pkg = resolvePackageData(pkgId, basedir)

if (!pkg) {
Expand Down Expand Up @@ -465,19 +481,55 @@ export function tryNodeResolve(

export function tryOptimizedResolve(
id: string,
server: ViteDevServer
server: ViteDevServer,
importer?: string
): string | undefined {
const cacheDir = server.config.cacheDir
const depData = server._optimizeDepsMetadata
if (cacheDir && depData) {
const isOptimized = depData.optimized[id]
if (isOptimized) {
return (
isOptimized.file +
`?v=${depData.browserHash}${
isOptimized.needsInterop ? `&es-interop` : ``
}`
)

if (!cacheDir || !depData) return

const getOptimizedUrl = (optimizedData: typeof depData.optimized[string]) => {
return (
optimizedData.file +
`?v=${depData.browserHash}${
optimizedData.needsInterop ? `&es-interop` : ``
}`
)
}

// check if id has been optimized
const isOptimized = depData.optimized[id]
if (isOptimized) {
return getOptimizedUrl(isOptimized)
}

if (!importer) return

// further check if id is imported by nested dependency
let resolvedSrc: string | undefined

for (const [pkgPath, optimizedData] of Object.entries(depData.optimized)) {
// check for scenarios, e.g.
// pkgPath => "my-lib > foo"
// id => "foo"
// this narrows the need to do a full resolve
if (!pkgPath.endsWith(id)) continue

// lazily initialize resolvedSrc
if (resolvedSrc == null) {
try {
// this may throw errors if unable to resolve, e.g. aliased id
resolvedSrc = resolveFrom(id, path.dirname(importer))
} catch {
// this is best-effort only so swallow errors
break
}
}

// match by src to correctly identify if id belongs to nested dependency
if (optimizedData.src === resolvedSrc) {
return getOptimizedUrl(optimizedData)
}
}
}
Expand Down
20 changes: 19 additions & 1 deletion packages/vite/src/node/utils.ts
Expand Up @@ -30,7 +30,11 @@ export function unwrapId(id: string): string {
return id.startsWith(VALID_ID_PREFIX) ? id.slice(VALID_ID_PREFIX.length) : id
}

export const flattenId = (id: string): string => id.replace(/[\/\.]/g, '_')
export const flattenId = (id: string): string =>
id.replace(/(\s*>\s*)/g, '__').replace(/[\/\.]/g, '_')

export const normalizeId = (id: string): string =>
id.replace(/(\s*>\s*)/g, ' > ')

export function isBuiltin(id: string): boolean {
return builtins.includes(id)
Expand All @@ -55,6 +59,20 @@ export function resolveFrom(id: string, basedir: string, ssr = false): string {
})
}

/**
* like `resolveFrom` but supports resolving `>` path in `id`,
* for example: `foo > bar > baz`
*/
export function nestedResolveFrom(id: string, basedir: string): string {
const pkgs = id.split('>').map((pkg) => pkg.trim())
try {
for (const pkg of pkgs) {
basedir = resolveFrom(pkg, basedir)
}
} catch {}
return basedir
}

// set in bin/vite.js
const filter = process.env.VITE_DEBUG_FILTER

Expand Down
15 changes: 15 additions & 0 deletions yarn.lock
Expand Up @@ -5575,6 +5575,13 @@ neo-async@^2.6.0:
resolved "https://registry.yarnpkg.com/neo-async/-/neo-async-2.6.2.tgz#b4aafb93e3aeb2d8174ca53cf163ab7d7308305f"
integrity sha512-Yd3UES5mWCSqR+qNT93S3UoYUkqAZ9lLg8a7g9rimsWmYGK8cVToA4/sF3RrshdyV3sAGMXVUmpMYOw+dLpOuw==

"nested-exclude@link:./packages/playground/optimize-deps/nested-exclude":
version "0.0.0"
uid ""

"nested-include@file:./packages/playground/optimize-deps/nested-exclude/nested-include":
version "1.0.0"

nice-try@^1.0.4:
version "1.0.5"
resolved "https://registry.yarnpkg.com/nice-try/-/nice-try-1.0.5.tgz#a3378a7696ce7d223e88fc9b764bd7ef1089e366"
Expand Down Expand Up @@ -7461,6 +7468,14 @@ test-exclude@^6.0.0:
version "0.0.0"
uid ""

"test-package-d-nested@link:./packages/playground/nested-deps/test-package-d/test-package-d-nested":
version "0.0.0"
uid ""

"test-package-d@link:./packages/playground/nested-deps/test-package-d":
version "0.0.0"
uid ""

text-extensions@^1.0.0:
version "1.9.0"
resolved "https://registry.yarnpkg.com/text-extensions/-/text-extensions-1.9.0.tgz#1853e45fee39c945ce6f6c36b2d659b5aabc2a26"
Expand Down