Skip to content

fix: remove misplaced /*#__PURE__*/ annotations#3072

Open
CemYil03 wants to merge 1 commit into
graphql-hive:masterfrom
CemYil03:fix/rolldown-invalid-pure-annotations
Open

fix: remove misplaced /*#__PURE__*/ annotations#3072
CemYil03 wants to merge 1 commit into
graphql-hive:masterfrom
CemYil03:fix/rolldown-invalid-pure-annotations

Conversation

@CemYil03

@CemYil03 CemYil03 commented Jun 8, 2026

Copy link
Copy Markdown

Description

Bundling graphql-scalars with Vite 6 / Rolldown produces hundreds of
[INVALID_ANNOTATION] warnings such as:

[INVALID_ANNOTATION] A comment "/*#__PURE__*/" in
"node_modules/graphql-scalars/esm/scalars/Byte.js" contains an annotation that
Rolldown cannot interpret due to the position of the comment.
   ╭─[ node_modules/graphql-scalars/esm/scalars/Byte.js:54:1 ]
54 │ /*#__PURE__*/ {
   │ ──────┬──────
   │       ╰──── comment ignored due to position
   │ Help: https://rolldown.rs/in-depth/dead-code-elimination#pure

Root cause. Per the
Rolldown DCE docs,
the /*#__PURE__*/ annotation is only honored when it sits immediately before a
function call or new expression. The repository placed annotations in
front of object literals (= /*#__PURE__*/ { ... }) and regex literals
(/*#__PURE__*/ /…/) in many scalars. esbuild/Terser silently ignored those
positions; Rolldown logs each one.

Fix. Drop only the misplaced annotations:

  • 18 scalar files where Config = /*#__PURE__*/ { … } becomes Config = { … }
    (BigInt, Byte, Cuid, Currency, EmailAddress, HSL, HexColorCode,
    Hexadecimal, LocalDateTime, NonNegativeFloat, NonNegativeInt, UUID,
    iso-date/Date, iso-date/DateTime, iso-date/DateTimeISO,
    iso-date/Duration, json/JSON, json/JSONObject).
  • src/scalars/PostalCode.ts (20 regex entries) and src/scalars/ObjectID.ts
    (1 regex).

Every annotation that actually does something is preserved — the
/*#__PURE__*/ new GraphQLScalarType(...) instantiations and the
/*#__PURE__*/ Object.assign(...) / /*#__PURE__*/ new RegExp(...) calls — so
dead-code elimination behavior is unchanged. The DCE benefit was always being
delivered by the annotation in front of the new GraphQLScalarType(Config)
call, not by the no-op annotation in front of the Config object literal.

No runtime change. No public API change. Patch-level changeset added.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as
    expected)
  • This change requires a documentation update

Screenshots/Sandbox (if appropriate/relevant):

Reproduction (before vs. after) using Rolldown directly against the built ESM
output:

// check.mjs
import { rolldown } from 'rolldown';
let count = 0;
const bundle = await rolldown({
  input: 'node_modules/graphql-scalars/esm/index.js',
  onLog: (_, log) => { if (log.code === 'INVALID_ANNOTATION') count++; },
});
await bundle.generate({ format: 'esm' });
console.log('INVALID_ANNOTATION warnings:', count);
await bundle.close();
  • Before: hundreds of warnings (one per misplaced annotation across
    Byte.js, Cuid.js, UUID.js, PostalCode.js, etc.).
  • After this PR: INVALID_ANNOTATION warnings: 0.

How Has This Been Tested?

  • yarn build — succeeds; bob single-build emits the package as before.
  • yarn jest --forceExit --no-watchman2797/2797 tests pass, 98 snapshots green.
  • grep -RnE '/\*#__PURE__\*/\s*[{/]' src/ — returns nothing (no
    /*#__PURE__*/ left in front of { or /).
  • grep -Rn '__PURE__' src/ — only matches in front of new,
    Object.assign, or new RegExp.
  • Reproduced the consumer-side warning (see Sandbox snippet above) against
    the rebuilt dist/esm and confirmed Rolldown reports 0
    INVALID_ANNOTATION warnings (down from hundreds).

Test Environment:

  • OS: macOS 26.5 (Darwin 25.5.0, arm64)
  • GraphQL Scalars Version: 1.25.0 (this branch)
  • NodeJS: v24.14.0

Checklist:

  • I have followed the
    CONTRIBUTING doc and the
    style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas — n/a, the change is purely
    removing comments that the consumer's bundler rejects
  • I have made corresponding changes to the documentation — n/a, no docs reference these
    annotations
  • My changes generate no new warnings (and remove a large class of consumer-side warnings)
  • I have added tests that prove my fix is effective or that my feature works — verified via the
    Rolldown reproduction above; the existing 2797-test suite remains green and is the
    behavioral guardrail
  • New and existing unit tests and linter rules pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules — n/a

Further comments

The annotations were not load-bearing where they were removed: in TypeScript,
the only side-effect-free expressions in these files are the new GraphQLScalarType(...) / Object.assign(...) / new RegExp(...) calls, and
those still carry their /*#__PURE__*/ comments. The annotations attached to
object literals and regex literals never produced any DCE benefit — they were
dead comments under esbuild/Terser too; Rolldown is simply louder about it.

Alternatives considered:

  1. Suppress the warning at the consumer level (Vite onwarn hook). Rejected
    — every downstream consumer would have to add the same workaround.
  2. Move the annotation to a wrapping IIFE (e.g.
    = /*#__PURE__*/ (() => ({ … }))()). Rejected — adds runtime cost and
    bytes for zero DCE gain over the existing annotation on the
    new GraphQLScalarType(...) call.
  3. Remove the noise at source (this PR). Smallest, safest change; ships a
    patch release that quiets every consumer's bundler.

Rolldown only honors /*#__PURE__*/ immediately before a function call or
`new` expression; annotations on object literals and regex literals
trigger [INVALID_ANNOTATION] warnings (esbuild/Terser silently ignored
them). Drop the misplaced annotations across scalar Configs and the
PostalCode/ObjectID regex tables. The valid annotations on
`new GraphQLScalarType(...)`, `Object.assign(...)`, and
`new RegExp(...)` are kept, so dead-code elimination is unchanged.
@CemYil03

Copy link
Copy Markdown
Author

@ardatan could you take a look please?

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant