feat(plugin-types): opt-in globals subpath for the host-injected URL global (SDK 3.0.1 prep) - #8
Merged
Merged
Conversation
…global - new src/globals.ts (declare-global module form) emitting dist/globals.d.ts via the normal tsc build; exports map + typesVersions gain "./globals" - optional-typed URL (URLConstructor | undefined) covering the epic's v1 subset incl. canParse, minus searchParams (docblock notes the runtime getter throws TypeError); Foundation semantics + pinned WHATWG divergences + minHostVersion guidance documented in the docblock - pin lib ES2020 in the package tsconfig (default ES2020 lib pulls in lib.dom, whose `var URL` would collide with the new global declaration) - 4 consumer fixtures + strict runner wired into `npm test`: reference-line opt-in typechecks guarded `new URL(...)`; types-array opt-in works; NO reference => only TS2304 'URL' (no leak from the main entry); lib.dom webview tsconfig unaffected (mutable href + searchParams intact) - reword the 3 "no ambient globals" claims (index.ts, README, docs-site installation.md) to "one opt-in globals subpath" + new teaching sections - index.ts header staged at 3.0.1; package.json versions left at 3.0.0 so the user-authorized `./release.sh 3.0.1` performs the lockstep bump, tag, and publish post-merge Task: fn-182-inject-foundation-bridged-url-global.3 Claude-Session: https://claude.ai/code/session_015NhVkmXAW9YUYMp6oumnwe
The TypeDoc tsconfig compiles all three packages' src trees under the
default full lib (DOM included — plugin-utils needs globalThis.crypto),
so plugin-types' new `declare global { var URL ... }` collided with
lib.dom's URL (TS2403) and broke the docs build. The subpath is
deliberately outside the API-reference entry point (index.ts), so
excluding it from this one program is the correct scope.
Caught by running the full docs-site build locally before opening the PR.
Task: fn-182-inject-foundation-bridged-url-global.3
Claude-Session: https://claude.ai/code/session_015NhVkmXAW9YUYMp6oumnwe
…n-types tests Review r1 fixes (codex NEEDS_WORK): - Major #1 (version contract inconsistency): stage the full lockstep 3.0.1 bump (3 workspaces + root + lockfile, via the exact release.sh command: `npm version 3.0.1 --workspaces --include-workspace-root --no-git-tag-version --allow-same-version`) so package.json matches the src/index.ts 3.0.1 header. release.sh's commit step now tolerates a pre-staged bump (skips the empty commit) so the user-authorized `./release.sh 3.0.1` remains the single release path: build + test + no-op bump + tag + publish + push. Publish still NOT performed here. - Major #2 (npm test not self-contained): plugin-types gains `"pretest": "npm run build"` so a clean checkout's `npm test` builds dist/ before the typetests + fixture gate (verified: rm -rf dist && root `npm test` green). The fixture runner's explicit missing-dist error stays as a backstop for direct `node fixtures/globals/run.mjs`. Task: fn-182-inject-foundation-bridged-url-global.3 Claude-Session: https://claude.ai/code/session_015NhVkmXAW9YUYMp6oumnwe
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0487dafe67
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
codex r2 Major: the guidance said minHostVersion permits unguarded use of the injected URL global, but the appos.jsc.urlGlobal.disabled kill switch can leave it undefined on ANY host version (as can menu-bar contexts), so a non-null assertion could crash at runtime. Reword all three guidance sites (src/globals.ts docblock, README opt-in section, docs-site installation.md) to require the typeof guard in all cases and scope minHostVersion to removing only the older-host reason for absence. Task: fn-182-inject-foundation-bridged-url-global.3 Claude-Session: https://claude.ai/code/session_015NhVkmXAW9YUYMp6oumnwe
- Replace fixture's bare-reference `if (URL)` guard with absence-safe typeof form; sweep found no other bare-reference examples - Mandate DOM-free lib as the only reliable isolation; stop presenting the lib.dom declaration conflict as a fail-loud safeguard (skipLibCheck merges silently) Claude-Session: https://claude.ai/code/session_015NhVkmXAW9YUYMp6oumnwe
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
SDK side of fn-182 (host-injected Foundation-bridged
URLglobal for the JSC plugin runtime): a new opt-in ambient-declaration subpath@appos.space/plugin-types/globals, plus the contract reword from "no ambient globals" to "one opt-in globals subpath".packages/plugin-types/src/globals.ts(new) — declare-global module form, so the normaltscbuild emitsdist/globals.d.ts. Declaresvar URL: URLConstructor | undefined+ theURL/URLConstructorinterfaces covering the epic's v1 subset:href/protocol/hostname/host/port/pathname/search/hash/origin/username/password/toString/toJSON+ staticcanParse— nosearchParams(type omits it; docblock notes the runtime getter throwsTypeError), noURL.parse, all accessors readonly."", out-of-range ports accepted,%3A→%253Adouble-encode on href round-trip, IPv6 hostname unbracketed withhost/originre-bracketing incl. the pinned literals); why the type is optional (older hosts / menu-bar contexts /appos.jsc.urlGlobal.disabledkill switch) andminHostVersionguidance; the never-throwscanParsecontract and the TypeError contract.package.json—exportsmap gains"./globals"(types-only) +typesVersionsfallback for legacynode10resolution;npm testnow also runs the consumer-fixture gate.tsconfig.json— pins"lib": ["ES2020"](the default ES2020 lib pulls in lib.dom, whosevar URLcollides with the new global inside the package's own build).fixtures/globals/(new, not published —filesisdist-only) — 4 standalone consumer tsconfigs resolving the package BY NAME through the workspace symlink + builtdist/, with a strict runner:jsc-with-globals— reference line opt-in; guardednew URL(...)typechecks; self-checking@ts-expect-errorfor unguarded use,searchParams, readonly assignment → clean compilejsc-types-array— tsconfigtypesarray opt-in → clean compilejsc-without-globals— no reference, main entry imported → MUST fail with ONLYTS2304 'URL'(no leak from the main entry)webview-dom— lib.dom tsconfig, no reference → clean compile with mutablehref+searchParams(browser URL untouched)src/index.ts:6,README.mdUsage lead-in, docs-siteinstallation.mdTypeScript-setup paragraph now say module exports by default + ONE opt-in globals subpath; README + installation.md gain teaching sections (opt-in forms, guard pattern, divergences pointer, webview warning).docs-site/tsconfig.typedoc.json— excludes the subpath from the TypeDoc program (deliberately outside theindex.tsAPI entry; the TypeDoc program needs full lib for plugin-utils'globalThis.crypto, so the global would collide with lib.dom's URL and break the docs build).Spec deviation (deliberate)
The task spec names the new file
src/globals.d.ts; it is authored assrc/globals.ts(declare-global module form) because tsc never emits input.d.tsfiles — a.tssource is the only way to satisfy the acceptance criterion "tsc emit produces dist/globals.d.ts" without adding a copy step. The epic explicitly allows "global-script or declare-global form".Release notes — v3.0.1 (publish is USER-authorized; not performed here)
@appos.space/plugin-types/globalssubpath declaring the host-injectedURLglobal (optional-typed; guard withtypeof URL === "function"or pinminHostVersionto an injecting host release — targeted host 1.1.0).src/index.ts's 3.0.1 header (codex review r1).release.sh's commit step now tolerates a pre-staged bump (skips the empty commit), so after merge the release is still one user-run command on main:./release.sh 3.0.1(builds, tests, no-op bump, tagsv3.0.1, publishes, pushes tags). If batching with fn-178's SDK change changes the number, re-run./release.sh <version>with that number — the bump step will move the versions forward normally.npm testself-contained viapretest; r2: require thetypeof URLguard unconditionally —minHostVersiondoes not override the kill switch or menu-bar limitation).Verification
npm run build+npm testgreen at root (59 runtime tests + typetests + the 4-fixture globals gate)docs-site npm run check-driftgreen (drift gate hashespermissions.ts/core.ts/schemas only — untouched)docs-site npm run build(Astro + TypeDoc + llms.txt) green locallyEpic: fn-182-inject-foundation-bridged-url-global · Task: .3
https://claude.ai/code/session_015NhVkmXAW9YUYMp6oumnwe