Skip to content

fix(typegpu): avoid Unicode property escape literals for ICU-less engines - #3255

Closed
aleclarson wants to merge 1 commit into
software-mansion:mainfrom
aleclarson:fix/no-icu-identifier-regexes
Closed

aleclarson wants to merge 1 commit into
software-mansion:mainfrom
aleclarson:fix/no-icu-identifier-regexes

Conversation

@aleclarson

Copy link
Copy Markdown

Summary

TypeGPU declares \p{XID_Start} / \p{XID_Continue} / \p{ID_*} regex literals at module scope (nameUtils.ts, core/resolve/externals.ts, minify.ts, shared/tseynit.ts). On a JavaScript engine without Unicode property data, every \p{…} escape is an early SyntaxError. Because it is a literal, the error happens at parse time and fails the whole module, even if the function holding the literal never runs. So simply importing typegpu (or a package that bundles it) crashes the app at startup.

This PR keeps the current behavior wherever ICU is available and lets TypeGPU load on engines without it:

  • New src/shared/identifierClasses.ts builds each property escape with new RegExp inside try/catch and returns an explicit-range fallback if the engine rejects it.
    • Strict fallbacks (ASCII) are used for checks where rejecting a valid identifier fails safely: validateIdentifier, and tseynit's JS-identifier test, which then JSON-quotes the key instead of emitting it bare.
    • Loose fallbacks treat every non-ASCII, non-blank code point as an identifier character. They are used for tokenizing (externals prop-chain matching, the minifier's separator check), because all of WGSL's non-identifier tokens are ASCII.
  • On ICU-capable engines every rewritten regex has a .source byte-identical to the original literal, so normal environments see no behavior change.

Failing runtime

NativeScript's embedded V8 on both iOS (@nativescript/ios 9.1.0) and Android (@nativescript/android 9.1.1, V8 14.9.207.39) is built without ICU. We measured this on a physical Android device (OnePlus CPH2551) and on an iOS simulator:

/\p{XID_Start}/u       -> SyntaxError: Invalid regular expression: /\p{XID_Start}/u: Invalid property name
/[_\p{XID_Start}]/u    -> SyntaxError: ... Invalid property name in character class
/\p{ID_Start}/u, /\p{L}/u, /\p{Script=Latin}/u -> same SyntaxError
typeof Intl            -> "undefined"

Every property escape is rejected, not only XID_*, so switching to \p{ID_Start} or \p{L} would not help. The same applies to node --without-intl and nodejs-mobile. We hit this through the shaders package, which bundles TypeGPU 0.12.3: importing it killed app startup on Android and iOS.

Tests

tests/identifierClasses.test.ts:

  • with ICU: the .source of wgslIdentRegex, anyIdent and boundedPropChain equals the original literals;
  • simulated ICU-less engine (a RegExp proxy that throws on \p{): the fallbacks are used, validateIdentifier accepts/rejects ASCII identifiers correctly, non-ASCII object keys are JSON-quoted, anyIdent matches $ñame, and minify keeps non-ASCII identifiers intact;
  • a source guard: no file under src/ other than shared/identifierClasses.ts contains a \p{…}/\P{…} escape.

Locally: the full typegpu test project passes (199 files / 2959 tests), plus tsc --noEmit, oxlint and oxfmt.

Copilot AI balanced review requested due to automatic review settings October 22, 2026 01:52

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.

🟢 Approval recommended

The focused regex replacements preserve supported-engine behavior, provide guarded fallbacks, and have no identified blocking defects.

No open issues 🎉

What changed in this PR

This PR prevents TypeGPU imports from failing on engines without Unicode property support by constructing identifier regexes dynamically with guarded fallbacks.

Changes:

  • Adds shared identifier classes with strict validation and loose tokenization fallbacks.
  • Replaces property-escape literals while preserving existing regex patterns on supported engines.
  • Adds compatibility tests and a source guard against reintroducing property escapes elsewhere.
File Description
packages/​typegpu/​tests/​identifierClasses.test.ts Tests supported and fallback behavior and guards source files.
packages/​typegpu/​src/​shared/​tseynit.ts Uses guarded identifier classes for object-key quoting.
packages/​typegpu/​src/​shared/​identifierClasses.ts Defines guarded Unicode property classes and fallback ranges.
packages/​typegpu/​src/​nameUtils.ts Uses the shared strict WGSL identifier regex.
packages/​typegpu/​src/​minify.ts Uses fallback-aware identifier checks when preserving separators.
packages/​typegpu/​src/​core/​resolve/​externals.ts Uses fallback-aware identifier and property-chain matching.

Review effort: Balanced

@pullfrog pullfrog Bot 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.

✅ No new issues found.

( ◦°^°◦) ✅

Ahoy! I combed the whole diff o' this PR, checked the fallback ranges against the ship's own whitespace list, ran the new test reel (4/4 passed), and found no leak in the hull. Green light to sail, matey!

Reviewed changes

  • shared/identifierClasses.ts (new) — builds every \p{…} escape through new RegExp inside a boundary-safe propertyEscape probe, falling back to explicit ranges when the engine rejects the property escape.
  • core/resolve/externals.ts, minify.ts, nameUtils.ts, shared/tseynit.ts — the four consumers now build their regexes from those runtime values instead of module-scope literals.
  • tests/identifierClasses.test.ts (new) — pins byte-identical .source on ICU engines, exercises the fallbacks via a RegExp proxy that throws on \p{, and adds a source guard that no other src/ file reintroduces a property-escape literal.

I verified the load-bearing assumptions independently: new RegExp('\\p{XID_Start}', 'u').source round-trips on an ICU engine, nonAsciiNonBlank excludes exactly the non-ASCII blanks listed in core/whitespaces.ts (exhaustive code-point check across U+0080–U+10FFFF, zero mismatches), and no \p{/\P{ literal survives anywhere in the repo.

ℹ️ Strict fallback turns some non-ASCII rejections into thrown errors on ICU-less engines

The stated rationale is that the strict (ASCII) fallbacks "fail safely" when they reject a valid identifier. That holds for sanitizePrimer/makeUniqueIdentifier, but not universally: validateProp callers in data/struct.ts:51 and data/autoStruct.ts:88 throw Invalid property key '…', and fnCore.ts:118 throws Invalid argument name "…" for string-implementation entry args. Since a non-ASCII name is valid WGSL but rejected by the ASCII fallback, these paths now error on ICU-less engines where they previously worked on ICU engines. This is not a regression — the prior behavior was a startup crash — and it follows from the deliberate strict/loose split, so no action is required; flagging only so the "fails safely" wording is understood to mean "throws a clear error" in those two paths rather than "silently sanitizes".

Technical details
# Strict fallback rejection paths on ICU-less engines

## Affected sites
- `packages/typegpu/src/data/struct.ts:49-52` — `validateProp(key)` throw on `d.struct({ 'ünï': … })`
- `packages/typegpu/src/data/autoStruct.ts:86-89` — same for auto-struct WGSL keys
- `packages/typegpu/src/core/function/fnCore.ts:116-121` — throw for non-ASCII positional arg names in string-implementation entry functions

## Required outcome
- None required. If a "sanitize instead of throw" behavior is ever wanted on ICU-less engines, the loose class would be the correct input to `validateIdentifier`; otherwise the current throw is acceptable and strictly better than the pre-PR parse-time crash.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

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.

2 participants