Skip to content

try to remove protojs pins#34393

Merged
freben merged 4 commits into
masterfrom
freben/protojs
May 26, 2026
Merged

try to remove protojs pins#34393
freben merged 4 commits into
masterfrom
freben/protojs

Conversation

@freben

@freben freben commented May 26, 2026

Copy link
Copy Markdown
Member

Summary

Removes the protobufjs/inquire version pins from the seed lockfile and suppresses the resulting bundler warning.

Background

@protobufjs/inquire@1.1.0 used eval("require")(moduleName) to hide a dynamic require from bundlers. When we pinned it, that was the workaround. The maintainer has since published 1.1.1 and 1.1.2 which remove the eval, but replace it with a bare require(moduleName) — still a dynamic require that webpack/rspack flags as "Critical dependency: the request of a dependency is an expression".

However, protobufjs 7.5.9 (2026-05-17, PR #2254) backported bundler-safe optional module lookups: all call sites that previously used util.inquire("fs"), util.inquire("buffer"), etc. were replaced with direct requires using webpack magic comments. The inquire utility is still in the dependency tree but is never exercised with dynamic module names.

What this PR does

  1. Removes the version pins for protobufjs and @protobufjs/inquire from packages/create-app/seed-yarn.lock, letting them resolve naturally (protobufjs 7.6.0, @protobufjs/inquire 1.1.2).

  2. Adds an ignoreWarnings entry in the bundler config (packages/cli-module-build) to suppress the false-positive "Critical dependency" warning from @protobufjs/inquire. The comment in the source explains the reasoning and links to the upstream issue.

Why ignoreWarnings is safe for all end users

The suppression ships to end users via @backstage/cli-module-build. This is safe regardless of which protobufjs version they have resolved in their lockfile:

  • @protobufjs/inquire is designed for optional module lookups — it returns null when a require fails, never crashes. The "Critical dependency" is a bundler static analysis warning, not a runtime error.
  • protobufjs is a server-side dependency (gRPC, Firestore, OpenTelemetry). If it ends up in a frontend bundle, the dynamic require was always non-functional regardless of version.
  • For protobufjs >= 7.5.9, the dynamic require path in inquire is dead code (never called).
  • For protobufjs < 7.5.9, the inquire calls degrade gracefully at runtime. We're not masking a real failure.

We considered adding "protobufjs": ">=7.5.9" as a dependency floor in a published package like @backstage/backend-defaults to force end users onto the fixed version. We decided against it — the ignoreWarnings is sufficient, and promoting a transitive dependency to a direct one just to set a floor could cause resolution conflicts for adopters with other constraints.

See also: protobufjs/protobuf.js#2057

Test plan

  • E2E create-app test passes (yarn install + tsc + build)
  • No protobufjs/inquire warnings in build output

🤖 Generated with Claude Code

Signed-off-by: Fredrik Adelöw <freben@gmail.com>
Copilot AI review requested due to automatic review settings May 26, 2026 08:30
@freben freben requested a review from a team as a code owner May 26, 2026 08:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot wasn't able to review any files in this pull request.

@backstage-goalie

backstage-goalie Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor

Missing Changesets

The following package(s) are changed by this PR but do not have a changeset:

  • @backstage/create-app

See CONTRIBUTING.md for more information about how to add changesets.

Changed Packages

Package Name Package Path Changeset Bump Current Version
@backstage/cli-module-build packages/cli-module-build patch v0.1.3
@backstage/create-app packages/create-app none v0.8.3

@backstage-goalie backstage-goalie Bot added size:tiny Tiny pull requests receive a higher priority for reviews. waiting-for:review The PR needs a review and will be visible in the review queue unless already assigned an owner. labels May 26, 2026
Since protobufjs 7.5.9, the @protobufjs/inquire utility is no longer
called with dynamic module names (replaced by bundler-safe lookups in
protobufjs/protobuf.js#2254). However, webpack/rspack still statically
analyzes the inquire source and emits a "Critical dependency" warning
for its require(moduleName) pattern. This is a false positive — the
code path is dead in practice.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Fredrik Adelöw <freben@gmail.com>
@freben freben requested a review from a team as a code owner May 26, 2026 09:23
@backstage-goalie backstage-goalie Bot added size:small Small pull requests receive a slightly higher priority for reviews. and removed size:tiny Tiny pull requests receive a higher priority for reviews. labels May 26, 2026
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Fredrik Adelöw <freben@gmail.com>
Copilot AI review requested due to automatic review settings May 26, 2026 09:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

Comment thread packages/cli-module-build/src/lib/bundler/config.ts Outdated
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Fredrik Adelöw <freben@gmail.com>

@Rugvip Rugvip left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Alright, seems like an exceptional enough breakage of an existing dependency to make this worthwhile

@backstage-goalie backstage-goalie Bot added waiting-for:merge The PR has been approved and is awaiting merge. and removed waiting-for:review The PR needs a review and will be visible in the review queue unless already assigned an owner. labels May 26, 2026
@freben freben merged commit 0f5286f into master May 26, 2026
31 checks passed
@freben freben deleted the freben/protojs branch May 26, 2026 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:small Small pull requests receive a slightly higher priority for reviews. waiting-for:merge The PR has been approved and is awaiting merge.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants