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

chore: upgrade rollup to 3.25.2 #13608

Merged
merged 9 commits into from Jun 25, 2023
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
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -81,7 +81,7 @@
"prettier": "2.8.8",
"resolve": "^1.22.2",
"rimraf": "^5.0.1",
"rollup": "^3.21.0",
"rollup": "^3.25.2",
"simple-git-hooks": "^2.8.1",
"tslib": "^2.5.3",
"tsx": "^3.12.7",
Expand Down
2 changes: 1 addition & 1 deletion packages/vite/package.json
Expand Up @@ -69,7 +69,7 @@
"dependencies": {
"esbuild": "^0.18.6",
"postcss": "^8.4.24",
"rollup": "^3.21.0"
"rollup": "^3.25.2"
},
"optionalDependencies": {
"fsevents": "~2.3.2"
Expand Down
80 changes: 46 additions & 34 deletions packages/vite/src/node/build.ts
Expand Up @@ -5,16 +5,17 @@ import type {
ExternalOption,
InputOption,
InternalModuleFormat,
LoggingFunction,
ModuleFormat,
OutputOptions,
Plugin,
RollupBuild,
RollupError,
RollupLog,
RollupOptions,
RollupOutput,
RollupWarning,
RollupWatcher,
WarningHandler,
WatcherOptions,
} from 'rollup'
import type { Terser } from 'dep-types/terser'
Expand Down Expand Up @@ -867,47 +868,58 @@ const dynamicImportWarningIgnoreList = [

export function onRollupWarning(
warning: RollupWarning,
warn: WarningHandler,
warn: LoggingFunction,
config: ResolvedConfig,
): void {
function viteWarn(warning: RollupWarning) {
if (warning.code === 'UNRESOLVED_IMPORT') {
const id = warning.id
const exporter = warning.exporter
// throw unless it's commonjs external...
if (!id || !/\?commonjs-external$/.test(id)) {
throw new Error(
`[vite]: Rollup failed to resolve import "${exporter}" from "${id}".\n` +
`This is most likely unintended because it can break your application at runtime.\n` +
`If you do want to externalize this module explicitly add it to\n` +
`\`build.rollupOptions.external\``,
)
}
}
const viteWarn: LoggingFunction = (warnLog) => {
let warning: string | RollupLog

if (
warning.plugin === 'rollup-plugin-dynamic-import-variables' &&
dynamicImportWarningIgnoreList.some((msg) =>
warning.message.includes(msg),
)
) {
return
if (typeof warnLog === 'function') {
warning = warnLog()
} else {
warning = warnLog
}

if (warningIgnoreList.includes(warning.code!)) {
return
}
if (typeof warning === 'object') {
if (warning.code === 'UNRESOLVED_IMPORT') {
const id = warning.id
const exporter = warning.exporter
// throw unless it's commonjs external...
if (!id || !/\?commonjs-external$/.test(id)) {
throw new Error(
`[vite]: Rollup failed to resolve import "${exporter}" from "${id}".\n` +
`This is most likely unintended because it can break your application at runtime.\n` +
`If you do want to externalize this module explicitly add it to\n` +
`\`build.rollupOptions.external\``,
)
}
}

if (warning.code === 'PLUGIN_WARNING') {
config.logger.warn(
`${colors.bold(
colors.yellow(`[plugin:${warning.plugin}]`),
)} ${colors.yellow(warning.message)}`,
)
return
if (
warning.plugin === 'rollup-plugin-dynamic-import-variables' &&
dynamicImportWarningIgnoreList.some((msg) =>
// @ts-expect-error warning is RollupLog
warning.message.includes(msg),
)
) {
return
}

if (warningIgnoreList.includes(warning.code!)) {
return
}

if (warning.code === 'PLUGIN_WARNING') {
config.logger.warn(
`${colors.bold(
colors.yellow(`[plugin:${warning.plugin}]`),
)} ${colors.yellow(warning.message)}`,
)
return
}
}

warn(warning)
warn(warnLog)
}

const tty = process.stdout.isTTY && !process.env.CI
Expand Down
9 changes: 7 additions & 2 deletions packages/vite/src/node/plugins/html.ts
Expand Up @@ -606,6 +606,7 @@ export function buildHtmlPlugin(config: ResolvedConfig): Plugin {

async generateBundle(options, bundle) {
const analyzedChunk: Map<OutputChunk, number> = new Map()
const inlineEntryChunk = new Set<string>()
const getImportedChunks = (
chunk: OutputChunk,
seen: Set<string> = new Set(),
Expand Down Expand Up @@ -826,8 +827,7 @@ export function buildHtmlPlugin(config: ResolvedConfig): Plugin {
})

if (chunk && canInlineEntry) {
// all imports from entry have been inlined to html, prevent rollup from outputting it
delete bundle[chunk.fileName]
inlineEntryChunk.add(chunk.fileName)
}

const shortEmitName = normalizePath(path.relative(config.root, id))
Expand All @@ -837,6 +837,11 @@ export function buildHtmlPlugin(config: ResolvedConfig): Plugin {
source: result,
})
}

for (const fileName of inlineEntryChunk) {
// all imports from entry have been inlined to html, prevent rollup from outputting it
delete bundle[fileName]
}
},
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/vite/src/node/plugins/importAnalysisBuild.ts
Expand Up @@ -221,7 +221,7 @@ export function buildImportAnalysisPlugin(config: ResolvedConfig): Plugin {
try {
imports = parseImports(source)[0]
} catch (e: any) {
this.error(e, e.idx)
this.error(e)
}

if (!imports.length) {
Expand Down Expand Up @@ -462,7 +462,7 @@ export function buildImportAnalysisPlugin(config: ResolvedConfig): Plugin {
try {
imports = parseImports(code)[0].filter((i) => i.d > -1)
} catch (e: any) {
this.error(e, e.idx)
this.error(e)
}

const s = new MagicString(code)
Expand Down
2 changes: 1 addition & 1 deletion packages/vite/src/node/server/middlewares/error.ts
Expand Up @@ -15,7 +15,7 @@ export function prepareError(err: Error | RollupError): ErrorPayload['err'] {
id: (err as RollupError).id,
frame: strip((err as RollupError).frame || ''),
plugin: (err as RollupError).plugin,
pluginCode: (err as RollupError).pluginCode,
pluginCode: (err as RollupError).pluginCode?.toString(),
Copy link
Member Author

@sun0day sun0day Jun 25, 2023

Choose a reason for hiding this comment

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

Not sure whether we should deal with non-(string|number)-type pluginCode and code.

Copy link
Member

Choose a reason for hiding this comment

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

I think that's fine for now 👍

loc: (err as RollupError).loc,
}
}
Expand Down
19 changes: 15 additions & 4 deletions packages/vite/src/node/server/pluginContainer.ts
Expand Up @@ -50,6 +50,7 @@ import type {
PartialResolvedId,
ResolvedId,
RollupError,
RollupLog,
PluginContext as RollupPluginContext,
SourceDescription,
SourceMap,
Expand Down Expand Up @@ -84,6 +85,8 @@ import { createPluginHookUtils } from '../plugins'
import { buildErrorMessage } from './middlewares/error'
import type { ModuleGraph } from './moduleGraph'

const noop = () => {}

export const ERR_CLOSED_SERVER = 'ERR_CLOSED_SERVER'

export function throwClosedServerError(): never {
Expand Down Expand Up @@ -186,6 +189,11 @@ export async function createPluginContainer(
rollupVersion,
watchMode: true,
},
debug: noop,
info: noop,
warn: noop,
// @ts-expect-error noop
error: noop,
}

function warnIncompatibleMethod(method: string, plugin: string) {
Expand Down Expand Up @@ -271,7 +279,7 @@ export async function createPluginContainer(
// active plugin in that pipeline can be tracked in a concurrency-safe manner.
// using a class to make creating new contexts more efficient
class Context implements PluginContext {
meta = minimalContext.meta
meta = minimalContext.meta!
ssr = false
_scan = false
_activePlugin: Plugin | null
Expand Down Expand Up @@ -377,10 +385,10 @@ export async function createPluginContainer(
}

warn(
e: string | RollupError,
e: string | RollupLog | (() => string | RollupLog),
position?: number | { column: number; line: number },
) {
const err = formatError(e, position, this)
const err = formatError(typeof e === 'function' ? e() : e, position, this)
const msg = buildErrorMessage(
err,
[colors.yellow(`warning: ${err.message}`)],
Expand All @@ -400,6 +408,9 @@ export async function createPluginContainer(
// the the error middleware.
throw formatError(e, position, this)
}

debug = noop
info = noop
}

function formatError(
Expand Down Expand Up @@ -493,7 +504,7 @@ export async function createPluginContainer(
}
}
if (code) {
err.frame = generateCodeFrame(code, err.loc)
err.frame = generateCodeFrame(`${code}`, err.loc)
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/vite/src/node/ssr/ssrManifestPlugin.ts
Expand Up @@ -42,7 +42,7 @@ export function ssrManifestPlugin(config: ResolvedConfig): Plugin {
try {
imports = parseImports(code)[0].filter((i) => i.n && i.d > -1)
} catch (e: any) {
this.error(e, e.idx)
this.error(e)
}
if (imports.length) {
for (let index = 0; index < imports.length; index++) {
Expand Down
6 changes: 5 additions & 1 deletion playground/html/__tests__/html.spec.ts
Expand Up @@ -150,7 +150,7 @@ describe.runIf(isBuild)('build', () => {
const countPreloadTags = _countTags.bind(this, 'link[rel=modulepreload]')

test('is inlined', async () => {
await page.goto(viteTestUrl + '/inline/shared-1.html?v=1')
await page.goto(viteTestUrl + '/inline/shared-2.html?v=1')
expect(await countScriptTags()).toBeGreaterThan(1)
expect(await countPreloadTags()).toBe(0)
})
Expand All @@ -162,6 +162,10 @@ describe.runIf(isBuild)('build', () => {
})

test('execution order when inlined', async () => {
await page.goto(viteTestUrl + '/inline/shared-1.html?v=1')
expect((await page.textContent('#output')).trim()).toBe(
'dep1 common dep2 dep3 shared',
)
await page.goto(viteTestUrl + '/inline/shared-2.html?v=1')
expect((await page.textContent('#output')).trim()).toBe(
'dep1 common dep2 dep3 shared',
Expand Down