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: decouple DevEnvironment and server #16460

Merged
Prev Previous commit
Next Next commit
feat: safeModulePaths and denyGlob out of the server
  • Loading branch information
patak-dev committed Apr 19, 2024
commit 225ddf079ef80a6161a2bc029869b4107e9d6112
2 changes: 0 additions & 2 deletions packages/vite/src/node/__tests__/external.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { fileURLToPath } from 'node:url'
import { describe, expect, test } from 'vitest'
import type { SSROptions } from '../ssr'
import { resolveConfig } from '../config'
import { createIsConfiguredAsExternal } from '../external'
import { Environment } from '../environment'
Expand All @@ -27,6 +26,5 @@ async function createIsExternal(external?: true) {
'serve',
)
const environment = new Environment('ssr', resolvedConfig)
console.log(environment.options)
return createIsConfiguredAsExternal(environment)
}
19 changes: 6 additions & 13 deletions packages/vite/src/node/plugins/asset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ import {
} from '../utils'
import { DEFAULT_ASSETS_INLINE_LIMIT, FS_PREFIX } from '../constants'
import { cleanUrl, withTrailingSlash } from '../../shared/utils'
import type { ViteDevServer } from '../server'
import type { DevEnvironment } from '../server/environment'

// referenceId is base64url but replaces - with $
export const assetUrlRE = /__VITE_ASSET__([\w$]+)__(?:\$_(.*?)__)?/g
Expand Down Expand Up @@ -142,7 +140,6 @@ const viteBuildPublicIdPrefix = '\0vite:asset:public'
*/
export function assetPlugin(config: ResolvedConfig): Plugin {
registerCustomMime()
let server: ViteDevServer

return {
name: 'vite:asset',
Expand All @@ -152,10 +149,6 @@ export function assetPlugin(config: ResolvedConfig): Plugin {
generatedAssets.set(config, new Map())
},

configureServer(_server) {
server = _server
},

resolveId(id) {
if (!config.assetsInclude(cleanUrl(id)) && !urlRE.test(id)) {
return
Expand Down Expand Up @@ -199,12 +192,12 @@ export function assetPlugin(config: ResolvedConfig): Plugin {
let url = await fileToUrl(id, config, this)

// Inherit HMR timestamp if this asset was invalidated
if (server) {
const environment = this.environment as DevEnvironment | undefined
const mod = environment?.moduleGraph.getModuleById(id)
if (mod && mod.lastHMRTimestamp > 0) {
url = injectQuery(url, `t=${mod.lastHMRTimestamp}`)
}
const environment = this.environment
const mod =
environment?.mode === 'dev' &&
environment?.moduleGraph.getModuleById(id)
if (mod && mod.lastHMRTimestamp > 0) {
url = injectQuery(url, `t=${mod.lastHMRTimestamp}`)
}

return {
Expand Down
2 changes: 2 additions & 0 deletions packages/vite/src/node/plugins/esbuild.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,8 @@ export function esbuildPlugin(config: ResolvedConfig): Plugin {

return {
name: 'vite:esbuild',
// TODO: Decouple server, the resolved config should be enough
// We may need a `configureWatcher` hook
configureServer(_server) {
server = _server
server.watcher
Expand Down
22 changes: 7 additions & 15 deletions packages/vite/src/node/plugins/importAnalysis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import { parse as parseJS } from 'acorn'
import type { Node } from 'estree'
import { findStaticImports, parseStaticImport } from 'mlly'
import { makeLegalIdentifier } from '@rollup/pluginutils'
import type { ViteDevServer } from '..'
import {
CLIENT_DIR,
CLIENT_PUBLIC_PATH,
Expand Down Expand Up @@ -55,6 +54,7 @@ import { getDepOptimizationConfig } from '../config'
import type { ResolvedConfig } from '../config'
import type { Plugin } from '../plugin'
import type { DevEnvironment } from '../server/environment'
import { addSafeModulePath } from '../server/middlewares/static'
import { shouldExternalize } from '../external'
import { optimizedDepNeedsInterop } from '../optimizer'
import {
Expand Down Expand Up @@ -140,7 +140,7 @@ function extractImportedBindings(
}

/**
* Server-only plugin that lexes, resolves, rewrites and analyzes url imports.
* Dev-only plugin that lexes, resolves, rewrites and analyzes url imports.
*
* - Imports are resolved to ensure they exist on disk
*
Expand Down Expand Up @@ -174,7 +174,6 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
const clientPublicPath = path.posix.join(base, CLIENT_PUBLIC_PATH)
const enablePartialAccept = config.experimental?.hmrPartialAccept
const matchAlias = getAliasPatternMatcher(config.resolve.alias)
let server: ViteDevServer

let _env: string | undefined
let _ssrEnv: string | undefined
Expand Down Expand Up @@ -205,19 +204,12 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
return {
name: 'vite:import-analysis',

configureServer(_server) {
server = _server
},

async transform(source, importer, options) {
// In a real app `server` is always defined, but it is undefined when
// running src/node/server/__tests__/pluginContainer.spec.ts

const ssr = options?.ssr === true
const environment = this.environment as DevEnvironment | undefined

if (!server || !environment) {
return null
const environment = this.environment as DevEnvironment | undefined
if (!environment) {
return
}

const moduleGraph = environment.moduleGraph
Expand Down Expand Up @@ -531,7 +523,7 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
// record as safe modules
// safeModulesPath should not include the base prefix.
// See https://github.com/vitejs/vite/issues/9438#issuecomment-1465270409
server?._safeModulesPath.add(fsPathFromUrl(stripBase(url, base)))
addSafeModulePath(config, fsPathFromUrl(stripBase(url, base)))

if (url !== specifier) {
let rewriteDone = false
Expand Down Expand Up @@ -623,7 +615,7 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
if (
!isDynamicImport &&
isLocalImport &&
config.server.preTransformRequests
environment.options.dev.preTransformRequests
) {
// pre-transform known direct imports
// These requests will also be registered in transformRequest to be awaited
Expand Down
12 changes: 9 additions & 3 deletions packages/vite/src/node/plugins/loadFallback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import type { ResolvedConfig } from '../config'
import type { Plugin } from '../plugin'
import type { ViteDevServer } from '../server'
import { extractSourcemapFromFile } from '../server/sourcemap'
import { isFileServingAllowed } from '../server/middlewares/static'
import { isFileLoadingAllowed } from '../server/middlewares/static'
import type { DevEnvironment } from '../server/environment'
import type { EnvironmentModuleNode } from '../server/moduleGraph'
import { ensureWatchedFile } from '../utils'
Expand All @@ -19,13 +19,19 @@ export const ERR_LOAD_PUBLIC_URL = 'ERR_LOAD_PUBLIC_URL'
* A plugin to provide build load fallback for arbitrary request with queries.
*/
export function loadFallbackPlugin(config: ResolvedConfig): Plugin {
let server: ViteDevServer
let server: ViteDevServer | undefined
return {
name: 'vite:load-fallback',
configureServer(_server) {
server = _server
},
async load(id, options) {
if (!server) {
// If the DevEnvironment is created as standalone, the configureServer server
// won't be called
return
}

const environment = this.environment as DevEnvironment
if (!environment) {
return
Expand All @@ -47,7 +53,7 @@ export function loadFallbackPlugin(config: ResolvedConfig): Plugin {
const file = cleanUrl(id)
if (
environment.options.nodeCompatible ||
isFileServingAllowed(file, server)
isFileLoadingAllowed(config, file) // Do we need fsPathFromId here?
) {
try {
code = await fsp.readFile(file, 'utf-8')
Expand Down
2 changes: 2 additions & 0 deletions packages/vite/src/node/publicUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,7 @@ export {
export { send } from './server/send'
export { createLogger } from './logger'
export { searchForWorkspaceRoot } from './server/searchRoot'

// TODO: export isFileLoadingAllowed?
export { isFileServingAllowed } from './server/middlewares/static'
export { loadEnv, resolveEnvPrefix } from './env'
67 changes: 21 additions & 46 deletions packages/vite/src/node/server/__tests__/pluginContainer.spec.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
import { beforeEach, describe, expect, it } from 'vitest'
import type { PartialResolvedId } from 'rollup'
import type { ViteDevServer } from 'vite'
import { describe, expect, it } from 'vitest'
import type { UserConfig } from '../../config'
import { resolveConfig } from '../../config'
import type { Plugin } from '../../plugin'
import type { PluginContainer } from '../pluginContainer'
import { createPluginContainer } from '../pluginContainer'
import { DevEnvironment } from '../environment'

describe('plugin container', () => {
Expand Down Expand Up @@ -43,7 +39,7 @@ describe('plugin container', () => {
},
}

const { pluginContainer, environment } = await getPluginContainer({
const environment = await getDevEnvironment({
plugins: [plugin],
})

Expand All @@ -53,15 +49,11 @@ describe('plugin container', () => {
)
expect(entryModule.meta).toEqual({ x: 1 })

const loadResult: any = await pluginContainer.load(entryUrl, {
environment,
})
const loadResult: any = await environment.pluginContainer.load(entryUrl)
expect(loadResult?.meta).toEqual({ x: 2 })

await pluginContainer.transform(loadResult.code, entryUrl, {
environment,
})
await pluginContainer.close()
await environment.pluginContainer.transform(loadResult.code, entryUrl)
await environment.pluginContainer.close()

expect(metaArray).toEqual([{ x: 1 }, { x: 2 }])
})
Expand Down Expand Up @@ -89,12 +81,12 @@ describe('plugin container', () => {
},
}

const { pluginContainer, environment } = await getPluginContainer({
const environment = await getDevEnvironment({
plugins: [plugin1, plugin2],
})

await environment.moduleGraph.ensureEntryFromUrl(entryUrl, false)
await pluginContainer.load(entryUrl, { environment })
await environment.pluginContainer.load(entryUrl)

expect.assertions(1)
})
Expand Down Expand Up @@ -135,12 +127,12 @@ describe('plugin container', () => {
},
}

const { pluginContainer, environment } = await getPluginContainer({
const environment = await getDevEnvironment({
plugins: [plugin1, plugin2],
})

await environment.moduleGraph.ensureEntryFromUrl(entryUrl, false)
await pluginContainer.load(entryUrl, { environment })
await environment.pluginContainer.load(entryUrl)

expect.assertions(2)
})
Expand Down Expand Up @@ -170,19 +162,14 @@ describe('plugin container', () => {
},
}

const { pluginContainer, environment } = await getPluginContainer({
const environment = await getDevEnvironment({
plugins: [plugin],
})
await environment.moduleGraph.ensureEntryFromUrl(entryUrl, false)
const loadResult: any = await pluginContainer.load(entryUrl, {
environment,
})
const result: any = await pluginContainer.transform(
const loadResult: any = await environment.pluginContainer.load(entryUrl)
const result: any = await environment.pluginContainer.transform(
loadResult.code,
entryUrl,
{
environment,
},
)
expect(result.code).equals('2')
})
Expand Down Expand Up @@ -210,29 +197,23 @@ describe('plugin container', () => {
},
}

const { pluginContainer, environment } = await getPluginContainer({
const environment = await getDevEnvironment({
plugins: [plugin],
})
await environment.moduleGraph.ensureEntryFromUrl(entryUrl, false)
const loadResult: any = await pluginContainer.load(entryUrl, {
environment,
})
const result: any = await pluginContainer.transform(
const loadResult: any = await environment.pluginContainer.load(entryUrl)
const result: any = await environment.pluginContainer.transform(
loadResult.code,
entryUrl,
{
environment,
},
)
expect(result.code).equals('3')
})
})
})

async function getPluginContainer(inlineConfig?: UserConfig): Promise<{
pluginContainer: PluginContainer
environment: DevEnvironment
}> {
async function getDevEnvironment(
inlineConfig?: UserConfig,
): Promise<DevEnvironment> {
const config = await resolveConfig(
{ configFile: false, ...inlineConfig },
'serve',
Expand All @@ -241,14 +222,8 @@ async function getPluginContainer(inlineConfig?: UserConfig): Promise<{
// @ts-expect-error This plugin requires a ViteDevServer instance.
config.plugins = config.plugins.filter((p) => !p.name.includes('pre-alias'))

const pluginContainer = await createPluginContainer(config)

const mockedServer = {
pluginContainer,
config,
}

const environment = new DevEnvironment(mockedServer as any, 'client')
const environment = new DevEnvironment('client', config)
await environment.init()

return { pluginContainer, environment }
return environment
}
Loading