Skip to content

👷 fix typecheck for unbuilt json-schema-to-typescript fork#148

Merged
bcaudan merged 2 commits into
mainfrom
bcaudan/json-to-schema-typescript
Jun 26, 2026
Merged

👷 fix typecheck for unbuilt json-schema-to-typescript fork#148
bcaudan merged 2 commits into
mainfrom
bcaudan/json-to-schema-typescript

Conversation

@bcaudan

@bcaudan bcaudan commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Motivation

The json-schema-to-typescript dependency is a GitHub fork that adds readonly support not yet in the official package. Because the fork has no prepare script, yarn never builds its dist/ during install, causing yarn typecheck to fail with Cannot find module 'json-schema-to-typescript'.

This mirrors how browser-sdk handles the same fork.

Changes

  • Add an ambient module declaration (scripts/lib/global.d.ts) so TypeScript can resolve the import without a built dist/
  • Switch generate-schema-types.ts to a lazy dynamic import that auto-builds the fork if dist/ is missing, with a module-level cache to avoid redundant builds across schemas

Test instructions

  1. Delete node_modules/json-schema-to-typescript/dist to simulate a fresh install state
  2. Run yarn typecheck — should pass without errors
  3. Run yarn json-schemas:generate — should auto-build the fork and regenerate types

Checklist

  • Tested locally (playground)
  • Added unit tests for this change.
  • Added e2e/integration tests for this change.
  • Updated related documentation.
  • Agentic code review findings addressed or explicitly dismissed.

- add ambient module declaration so typecheck passes without dist
- lazy-load compile fn with build-on-demand fallback
@bcaudan bcaudan marked this pull request as ready for review June 25, 2026 07:47
@bcaudan bcaudan requested a review from a team as a code owner June 25, 2026 07:47
Copilot AI review requested due to automatic review settings June 25, 2026 07:47

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

This PR addresses TypeScript resolution/runtime issues caused by using an unbuilt GitHub fork of json-schema-to-typescript (no prepare, so dist/ may be missing after install), by adding a local ambient module declaration and making the schema generation script dynamically import (and build) the fork on demand.

Changes:

  • Add an ambient declare module 'json-schema-to-typescript' to allow yarn typecheck to succeed even when the fork’s dist/ isn’t present.
  • Update scripts/generate-schema-types.ts to lazily import('json-schema-to-typescript'), auto-building the fork when the import initially fails, and caching the resolved compile function.

Reviewed changes

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

File Description
scripts/lib/global.d.ts Adds a local ambient module declaration for json-schema-to-typescript to unblock typechecking when the fork isn’t built.
scripts/generate-schema-types.ts Switches to a cached dynamic import and adds a fallback build step when the module isn’t importable due to missing build output.

Comment thread scripts/generate-schema-types.ts
Comment thread scripts/generate-schema-types.ts
@bcaudan bcaudan requested a review from cdn34dd June 25, 2026 09:11

@cdn34dd cdn34dd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is there any news if this will land at some point so we can get rid of the github fork ? I looked into it, and found this PR from you from 2020 (bcherny/json-schema-to-typescript#353) :D.

But if I understood the issue correctly we get the issue due to the fork not having a dist/ folder because it's not a published package, and the fix is to lazy load the module instead of statically , so if dist/ is missing we build the fork before trying it import correct ? But how did we not notice this for so long ?

@bcaudan

bcaudan commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator Author

Is there any news if this will land at some point so we can get rid of the github fork ?

No, no news.

how did we not notice this for so long ?

I have stumbled upon it from time to time when starting a new worktree, I never took the time to do something about it before.

@bcaudan bcaudan merged commit ea8b1bb into main Jun 26, 2026
16 checks passed
@bcaudan bcaudan deleted the bcaudan/json-to-schema-typescript branch June 26, 2026 08:55
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.

3 participants