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: default esm SSR build, simplified externalization #8348

Merged
merged 13 commits into from May 29, 2022
2 changes: 1 addition & 1 deletion docs/config/ssr-options.md
Expand Up @@ -20,7 +20,7 @@ Prevent listed dependencies from being externalized for SSR. If `true`, no depen

## ssr.target

- **Type:** `'node' | 'webworker'`
- **Type:** `'node' | 'webworker' | 'node-cjs'`
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also use build.rollupOptions.output.format === 'cjs', but we need this target during dev for the new externalization logic. We could add a new option to switch externalization logic if not. ssr.lazy: true? And then people can choose to go back to the old eager collection with ssr.lazy: false?

Copy link
Collaborator

@benmccann benmccann May 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could the externalization logic also look at build.rollupOptions.output.format? Having two options seems a bit dangerous because someone could change build.rollupOptions.output.format, but not ssr.target

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could, since we want to promote the ESM build and the new externalization logic, we don't really have a new Vite option here... it is more like a escape hatch that we're leaving for people that can yet use the ESM build. If we do that, we should remove the new 'node-cjs' option in ssr.target and let users control the format and eager externalization using build.rollupOptions.output.format: 'cjs'.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same opinion as Ben: a single source of truth would be nice and build.rollupOptions.output.format seems like a natural fit.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you can use build.rollupOptions.output specifically, since it may be an array?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, and there are other issues with it, like needing to conditionally change it based on the ssr flag. But this flag isn't currently passed to the ConfigEnv. We could add it so we could configure using ({ mode, command, ssr }) => { ... }), but forcing a conditional as the only way to configure this doesn't seem very appealing. For other options, we discussed previously with @aleclarson and tried to avoid having SSR only equivalents, but this is a big global switch. Given that we already have ssr.target as an enum, I think that adding a node-cjs or cjs is ok. Although another option like ssr.format: 'esm' | 'cjs' could be easier to deprecate in the future... we could add it as @experimental, and give us room to remove it later on.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to ssr.format: 'cjs', we'll later review all these new options together (there is also ssr.scan and ssr.disabled: 'build' for example)
See #8348 (comment)

- **Default:** `node`

Build target for the SSR server.
7 changes: 7 additions & 0 deletions docs/vite.config.ts
@@ -0,0 +1,7 @@
import { defineConfig } from 'vite'

export default defineConfig({
ssr: {
target: 'node-cjs'
}
})
83 changes: 50 additions & 33 deletions packages/vite/src/node/build.ts
Expand Up @@ -31,7 +31,10 @@ import { manifestPlugin } from './plugins/manifest'
import type { Logger } from './logger'
import { dataURIPlugin } from './plugins/dataUri'
import { buildImportAnalysisPlugin } from './plugins/importAnalysisBuild'
import { resolveSSRExternal, shouldExternalizeForSSR } from './ssr/ssrExternal'
import {
cjsShouldExternalizeForSSR,
cjsSsrResolveExternals
} from './ssr/ssrExternal'
import { ssrManifestPlugin } from './ssr/ssrManifestPlugin'
import type { DepOptimizationMetadata } from './optimizer'
import {
Expand Down Expand Up @@ -373,27 +376,14 @@ async function doBuild(
ssr ? config.plugins.map((p) => injectSsrFlagToHooks(p)) : config.plugins
) as Plugin[]

// inject ssrExternal if present
const userExternal = options.rollupOptions?.external
let external = userExternal
if (ssr) {
// see if we have cached deps data available
let knownImports: string[] | undefined
const dataPath = path.join(getDepsCacheDir(config), '_metadata.json')
try {
const data = JSON.parse(
fs.readFileSync(dataPath, 'utf-8')
) as DepOptimizationMetadata
knownImports = Object.keys(data.optimized)
} catch (e) {}
if (!knownImports) {
// no dev deps optimization data, do a fresh scan
knownImports = await findKnownImports(config)
}
external = resolveExternal(
resolveSSRExternal(config, knownImports),
userExternal
)

// In CJS, we can pass the externals to rollup as is. In ESM, we need to
// do it in the resolve plugin so we can add the resolved extension for
// deep node_modules imports
if (ssr && config.ssr?.target === 'node-cjs') {
external = await cjsSsrResolveExternal(config, userExternal)
}

if (isDepsOptimizerEnabled(config) && !ssr) {
Expand Down Expand Up @@ -431,10 +421,12 @@ async function doBuild(

try {
const buildOutputOptions = (output: OutputOptions = {}): OutputOptions => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if the input is .mjs we still emits .js. I am not sure if we should always emit .mjs on ESM build, or respect the input (might need to also convert .mts)

Copy link
Member Author

@patak-dev patak-dev May 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should do the same we are doing in v3 for lib mode?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll merge this one and work on this logic in another PR.

const cjsSsrBuild = ssr && config.ssr?.target === 'node-cjs'
return {
dir: outDir,
format: ssr ? 'cjs' : 'es',
exports: ssr ? 'named' : 'auto',
// Default format is 'es' for regular and for SSR builds
format: cjsSsrBuild ? 'cjs' : 'es',
exports: cjsSsrBuild ? 'named' : 'auto',
sourcemap: options.sourcemap,
name: libOptions ? libOptions.name : undefined,
generatedCode: 'es2015',
Expand Down Expand Up @@ -696,26 +688,51 @@ export function onRollupWarning(
}
}

function resolveExternal(
ssrExternals: string[],
async function cjsSsrResolveExternal(
config: ResolvedConfig,
user: ExternalOption | undefined
): ExternalOption {
): Promise<ExternalOption> {
// see if we have cached deps data available
let knownImports: string[] | undefined
const dataPath = path.join(getDepsCacheDir(config), '_metadata.json')
try {
const data = JSON.parse(
fs.readFileSync(dataPath, 'utf-8')
) as DepOptimizationMetadata
knownImports = Object.keys(data.optimized)
} catch (e) {}
if (!knownImports) {
// no dev deps optimization data, do a fresh scan
knownImports = await findKnownImports(config)
}
const ssrExternals = cjsSsrResolveExternals(config, knownImports)

return (id, parentId, isResolved) => {
if (shouldExternalizeForSSR(id, ssrExternals)) {
const isExternal = cjsShouldExternalizeForSSR(id, ssrExternals)
if (isExternal) {
return true
}
if (user) {
if (typeof user === 'function') {
return user(id, parentId, isResolved)
} else if (Array.isArray(user)) {
return user.some((test) => isExternal(id, test))
} else {
return isExternal(id, user)
}
return resolveUserExternal(user, id, parentId, isResolved)
}
}
}

function resolveUserExternal(
user: ExternalOption,
id: string,
parentId: string | undefined,
isResolved: boolean
) {
if (typeof user === 'function') {
return user(id, parentId, isResolved)
} else if (Array.isArray(user)) {
return user.some((test) => isExternal(id, test))
} else {
return isExternal(id, user)
}
}

function isExternal(id: string, test: string | RegExp) {
if (typeof test === 'string') {
return id === test
Expand Down
2 changes: 1 addition & 1 deletion packages/vite/src/node/config.ts
Expand Up @@ -228,7 +228,7 @@ export interface ExperimentalOptions {
importGlobRestoreExtension?: boolean
}

export type SSRTarget = 'node' | 'webworker'
export type SSRTarget = 'node' | 'webworker' | 'node-cjs'

export interface SSROptions {
external?: string[]
Expand Down
14 changes: 9 additions & 5 deletions packages/vite/src/node/plugins/importAnalysis.ts
Expand Up @@ -41,7 +41,10 @@ import {
} from '../utils'
import type { ResolvedConfig } from '../config'
import type { Plugin } from '../plugin'
import { shouldExternalizeForSSR } from '../ssr/ssrExternal'
import {
cjsShouldExternalizeForSSR,
shouldExternalizeForSSR
} from '../ssr/ssrExternal'
import { transformRequest } from '../server/transformRequest'
import {
getDepsCacheDir,
Expand Down Expand Up @@ -359,10 +362,11 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
}
// skip ssr external
if (ssr) {
if (
server._ssrExternals &&
shouldExternalizeForSSR(specifier, server._ssrExternals)
) {
if (config.ssr?.target === 'node-cjs') {
if (cjsShouldExternalizeForSSR(specifier, server._ssrExternals)) {
continue
}
} else if (shouldExternalizeForSSR(specifier, config)) {
continue
}
if (isBuiltin(specifier)) {
Expand Down
7 changes: 6 additions & 1 deletion packages/vite/src/node/plugins/index.ts
Expand Up @@ -3,6 +3,7 @@ import type { ResolvedConfig } from '../config'
import { isDepsOptimizerEnabled } from '../config'
import type { Plugin } from '../plugin'
import { getDepsOptimizer } from '../optimizer'
import { shouldExternalizeForSSR } from '../ssr/ssrExternal'
import { jsonPlugin } from './json'
import { resolvePlugin } from './resolve'
import { optimizedDepsBuildPlugin, optimizedDepsPlugin } from './optimizedDeps'
Expand Down Expand Up @@ -61,7 +62,11 @@ export async function resolvePlugins(
packageCache: config.packageCache,
ssrConfig: config.ssr,
asSrc: true,
getDepsOptimizer: () => getDepsOptimizer(config)
getDepsOptimizer: () => getDepsOptimizer(config),
shouldExternalize:
isBuild && config.build.ssr && config.ssr?.target !== 'node-cjs'
? (id) => shouldExternalizeForSSR(id, config)
: undefined
}),
htmlInlineProxyPlugin(config),
cssPlugin(config),
Expand Down
44 changes: 35 additions & 9 deletions packages/vite/src/node/plugins/resolve.ts
Expand Up @@ -83,6 +83,7 @@ export interface InternalResolveOptions extends ResolveOptions {
scan?: boolean
// Resolve using esbuild deps optimization
getDepsOptimizer?: () => DepsOptimizer | undefined
shouldExternalize?: (id: string) => boolean | undefined
}

export function resolvePlugin(baseOptions: InternalResolveOptions): Plugin {
Expand All @@ -105,6 +106,7 @@ export function resolvePlugin(baseOptions: InternalResolveOptions): Plugin {
const depsOptimizer = baseOptions.getDepsOptimizer?.()

const ssr = resolveOpts?.ssr === true

if (id.startsWith(browserExternalId)) {
return id
}
Expand Down Expand Up @@ -258,7 +260,10 @@ export function resolvePlugin(baseOptions: InternalResolveOptions): Plugin {

// bare package imports, perform node resolve
if (bareImportRE.test(id)) {
const external = options.shouldExternalize?.(id)

if (
!external &&
asSrc &&
depsOptimizer &&
!ssr &&
Expand All @@ -270,7 +275,13 @@ export function resolvePlugin(baseOptions: InternalResolveOptions): Plugin {

if (
targetWeb &&
(res = tryResolveBrowserMapping(id, importer, options, false))
(res = tryResolveBrowserMapping(
id,
importer,
options,
false,
external
))
) {
return res
}
Expand All @@ -282,7 +293,8 @@ export function resolvePlugin(baseOptions: InternalResolveOptions): Plugin {
options,
targetWeb,
depsOptimizer,
ssr
ssr,
external
))
) {
return res
Expand Down Expand Up @@ -523,7 +535,8 @@ export function tryNodeResolve(
options: InternalResolveOptions,
targetWeb: boolean,
depsOptimizer?: DepsOptimizer,
ssr?: boolean
ssr?: boolean,
externalize?: boolean
): PartialResolvedId | undefined {
const { root, dedupe, isBuild, preserveSymlinks, packageCache } = options

Expand Down Expand Up @@ -591,7 +604,8 @@ export function tryNodeResolve(

let resolveId = resolvePackageEntry
let unresolvedId = pkgId
if (unresolvedId !== nestedPath) {
const isDeepImport = unresolvedId !== nestedPath
if (isDeepImport) {
resolveId = resolveDeepImport
unresolvedId = '.' + nestedPath.slice(pkgId.length)
}
Expand All @@ -616,15 +630,25 @@ export function tryNodeResolve(
return
}

const processResult = (resolved: PartialResolvedId) => {
if (!externalize) {
return resolved
}
const resolvedExt = path.extname(resolved.id)
const resolvedId =
isDeepImport && path.extname(id) !== resolvedExt ? id + resolvedExt : id
return { ...resolved, id: resolvedId, external: true }
}

// link id to pkg for browser field mapping check
idToPkgMap.set(resolved, pkg)
if (isBuild && !depsOptimizer) {
if ((isBuild && !depsOptimizer) || externalize) {
// Resolve package side effects for build so that rollup can better
// perform tree-shaking
return {
return processResult({
id: resolved,
moduleSideEffects: pkg.hasSideEffects(resolved)
}
})
}

if (
Expand Down Expand Up @@ -940,7 +964,8 @@ function tryResolveBrowserMapping(
id: string,
importer: string | undefined,
options: InternalResolveOptions,
isFilePath: boolean
isFilePath: boolean,
externalize?: boolean
) {
let res: string | undefined
const pkg = importer && idToPkgMap.get(importer)
Expand All @@ -953,10 +978,11 @@ function tryResolveBrowserMapping(
isDebug &&
debug(`[browser mapped] ${colors.cyan(id)} -> ${colors.dim(res)}`)
idToPkgMap.set(res, pkg)
return {
const result = {
id: res,
moduleSideEffects: pkg.hasSideEffects(res)
}
return externalize ? { ...result, external: true } : result
}
} else if (browserMappedPath === false) {
return browserExternalId
Expand Down
2 changes: 2 additions & 0 deletions packages/vite/src/node/plugins/ssrRequireHook.ts
Expand Up @@ -12,8 +12,10 @@ import { arraify } from '../utils'
export function ssrRequireHookPlugin(config: ResolvedConfig): Plugin | null {
if (
config.command !== 'build' ||
!config.build.ssr ||
!config.resolve.dedupe?.length ||
config.ssr?.noExternal === true ||
config.ssr?.target !== 'node-cjs' ||
isBuildOutputEsm(config)
) {
return null
Expand Down
4 changes: 2 additions & 2 deletions packages/vite/src/node/server/index.ts
Expand Up @@ -22,7 +22,7 @@ import {
resolveHostname
} from '../utils'
import { ssrLoadModule } from '../ssr/ssrModuleLoader'
import { resolveSSRExternal } from '../ssr/ssrExternal'
import { cjsSsrResolveExternals } from '../ssr/ssrExternal'
import {
rebindErrorStacktrace,
ssrRewriteStacktrace
Expand Down Expand Up @@ -330,7 +330,7 @@ export async function createServer(
...Object.keys(depsOptimizer.metadata.discovered)
]
}
server._ssrExternals = resolveSSRExternal(config, knownImports)
server._ssrExternals = cjsSsrResolveExternals(config, knownImports)
}
return ssrLoadModule(
url,
Expand Down