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 11 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions packages/playground/dynamic-import/css/index.css
Original file line number Diff line number Diff line change
@@ -1,2 +1,6 @@
.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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default 'D-nested@1.0.0'
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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/node_modules/test-package-d-nested'
],
exclude: ['test-package-d']
}
}
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export { default as nestedInclude } from 'nested-include'

export default 'nested-exclude'
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// written in cjs, optimization should convert this to esm
module.exports = 'nested-include'
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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/node_modules/nested-include'
],
exclude: ['nested-exclude'],
esbuildOptions: {
plugins: [
{
Expand Down
4 changes: 2 additions & 2 deletions packages/vite/src/node/plugins/preAlias.ts
Original file line number Diff line number Diff line change
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
67 changes: 59 additions & 8 deletions packages/vite/src/node/plugins/resolve.ts
Original file line number Diff line number Diff line change
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,16 @@ 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 "node_modules" to get actual nested package name
const nodeModulesMatch = id.match(/(.+)\/node_modules\/(.+)/)
const nestedRoot = nodeModulesMatch ? nodeModulesMatch[1] : ''
const nestedPath = nodeModulesMatch ? nodeModulesMatch[2] : id

// 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 +414,11 @@ export function tryNodeResolve(
basedir = root
}

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

const pkg = resolvePackageData(pkgId, basedir)

if (!pkg) {
Expand Down Expand Up @@ -465,20 +479,57 @@ 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) {

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

// check if id has been optimized
if (isOptimized) {
return getOptimizedUrl(isOptimized)
}

// if has importer, further check if id is importing from nested depedency
if (importer) {
let resolvedSrc: string | undefined
for (const [k, v] of Object.entries(depData.optimized)) {
Shinigami92 marked this conversation as resolved.
Show resolved Hide resolved
// check for scenarios, e.g.
// k => "my-lib/node_modules/foo"
// id => "foo"
// this narrows the need to do a full resolve
if (k.endsWith(id)) {
// 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 path, return url similar as above
if (v.src === resolvedSrc) {
return getOptimizedUrl(v)
}
}
}
}
}
}

Expand Down
16 changes: 15 additions & 1 deletion packages/vite/src/node/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ 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(/\/node_modules\//g, '__').replace(/[\/\.]/g, '_')

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

/**
* like `resolveFrom` but supports resolving `node_modules` path in `id`
*/
export function nestedResolveFrom(id: string, basedir: string): string {
const pkgs = id.split('/node_modules/')
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
Original file line number Diff line number Diff line change
Expand Up @@ -5425,6 +5425,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 @@ -7351,6 +7358,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