Skip to content

feat(performance): add the analyzableBailouts hint - #9

Open
anurag6569201 wants to merge 1 commit into
qa/agent-webpack-webpack/pr-09-21986/basefrom
qa/agent-webpack-webpack/pr-09-21986/head
Open

anurag6569201 wants to merge 1 commit into
qa/agent-webpack-webpack/pr-09-21986/basefrom
qa/agent-webpack-webpack/pr-09-21986/head

Conversation

@anurag6569201

Copy link
Copy Markdown

Summary

A reference in ESM output that keeps webpack's runtime form was reported only through stats.optimizationBailout, which nothing shows by default, so a user whose output silently stays unanalyzable has no way to learn why. performance.analyzableBailouts names each reason and the modules it was recorded on, through the same hints channel as the other checks. The reasons a module records are stored beside its generated code and reapplied when the cache restores it, so a warm build reports what a cold one did; the cache suite covers that.

What kind of change does this PR introduce?

feat

Did you add tests for your changes?

Yes — test/configCases/performance/analyzable-bailouts{,-error,-stats,-no-hints,-clean} (each reporting channel, plus a build where every reference bakes), and the updated Cli and Defaults snapshots.

Does this PR introduce a breaking change?

No; the option is off by default.

If relevant, what needs to be documented once your changes are merged or what have you already documented?

performance.analyzableBailouts on the performance page, alongside the other hints, with the list of reasons the runtime template can record and what lifts each.

Use of AI

AI (Claude Code) was used: it proposed the hint after auditing what the analyzable output still leaves in the runtime form, identified the code-generation cache as the reason a recorded reason alone would not survive a warm build, implemented the option through every config layer with the five cases, and ran the targeted suites and lint. I reviewed the design and the diff.

🤖 Generated with Claude Code

https://claude.ai/code/session_0187hRzHd96HJAoKe4bRF67Q


Generated by Claude Code

Summary by CodeRabbit

  • New Features

    • Added an optional performance check for ESM output that identifies references preventing literal import() or new URL() generation.
    • Reports detected issues as warnings, errors, or statistics hints according to the configured performance hint level.
    • Diagnostic messages include bailout reasons and affected modules to clarify runtime-form output.
    • Added the performance.analyzableBailouts configuration option.
  • Tests

    • Added coverage for warning, error, statistics, clean-output, and disabled-hints scenarios.

Source merge-base: 22dd9bb71f4e9e7d495a6a99a11a806818d9ed8b
Source head: b65cb952673224254e9e7073be53d7fc1ef4740a

@shipwright-agent

Copy link
Copy Markdown

⛔ Shipwright · Blocked

Recommendation: do not merge PR #9 · Tier T2
Checks: 0 total · 0 needing attention

Next step: resolve the blocking findings before merge.

Findings (7)

  • CRITICAL The finally block in the cached-result path calls finishAnalyzableBailouts() unconditionally, but the non-cached path already consumed the sink and stored it in result.data. · lib/Compilation.js:4399
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • CRITICAL The cached-result branch reads result.data.get("analyzableBailouts") and calls restoreAnalyzableBailouts(module, bailouts), but the non-cached branch stores the sink under result.d · lib/Compilation.js:4399
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • HIGH The method name 'analyzableBailoutsOf' is ambiguous: it reads from the moduleGraph's optimizationBailout array and filters by a string prefix, but the prefix constant is defined at · lib/RuntimeTemplate.js:119
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • HIGH The 'beginAnalyzableBailouts'/'finishAnalyzableBailouts' API is stateful and requires callers to remember to call finish in a finally block. · lib/Compilation.js:4340
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • HIGH The '_analyzableBailoutSink' is a single shared mutable array on RuntimeTemplate. · lib/RuntimeTemplate.js:492
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • HIGH The warning message includes module identifiers and reasons derived from code generation, but there is no sanitization or length limit on the reason strings or module names. · lib/errors/AnalyzableBailoutsWarning.js:20
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
  • HIGH The plugin collects bailouts from all modules and runtime modules into a Map keyed by reason string, with no cap on the number of unique reasons or total references. · lib/performance/AnalyzableBailoutsPlugin.js:48
    • Fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

Fireworks usage: 40,492 input · 1,030 output · 41,522 total tokens · $0.0096 · 17s · 0 fix iteration(s)

Open the Shipwright check for full evidence and the audit bundle. Use /shipwright rerun to verify again.

class AnalyzableBailoutsWarning extends WebpackError {
/**
* Creates an instance of AnalyzableBailoutsWarning.
* @param {BailoutDetails[]} bailouts the reasons found, the most frequent first

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shipwright · HIGH

The warning message includes module identifiers and reasons derived from code generation, but there is no sanitization or length limit on the reason strings or module names.

Impact: The warning message includes module identifiers and reasons derived from code generation, but there is no sanitization or length limit on the reason strings or module names. A malicious or pathological module name (e.g., containing control characters or extremely long strings) could produce a warning that breaks log parsing, terminal rendering, or downstream tooling that consumes stats output.

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.


// Nothing is recorded outside ESM output, so there is nothing to walk.
if (!runtimeTemplate.isModule()) return;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shipwright · HIGH

The plugin collects bailouts from all modules and runtime modules into a Map keyed by reason string, with no cap on the number of unique reasons or total references.

Impact: The plugin collects bailouts from all modules and runtime modules into a Map keyed by reason string, with no cap on the number of unique reasons or total references. In a large build with many distinct bailout reasons, this can cause unbounded memory growth and a very large warning message, potentially exhausting memory or producing output that is impractical to consume.

Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant