Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix schema generation in new TypeScript version #274

Merged
merged 2 commits into from
May 23, 2020

Conversation

jlfwong
Copy link
Owner

@jlfwong jlfwong commented May 23, 2020

Fixes #268

I fixed it by dropping the dependency on quicktype entirely, and using its dependency directly. I still don't understand why the version of typescript used in this repository affects what quicktype is doing, but it seems like the issue is in quicktype, not its dependency.

I validated this change was correct by diffing the output of node scripts/generate-file-format-schema-json.js with what's currently on http://speedscope.app/file-format-schema.json. There's no difference.

This PR also includes changes to the CI script to ensure that we can catch this before hitting master next time.

@coveralls
Copy link

coveralls commented May 23, 2020

Coverage Status

Coverage increased (+0.1%) to 47.107% when pulling d650058 on jlfwong/fix-schema-generation into 2077a90 on master.

@jlfwong jlfwong merged commit ca1abfd into master May 23, 2020
@jlfwong jlfwong deleted the jlfwong/fix-schema-generation branch May 23, 2020 23:08
jackerghan pushed a commit to jackerghan/speedscope that referenced this pull request Jul 28, 2023
Fixes jlfwong#268 

I fixed it by dropping the dependency on quicktype entirely, and using its dependency directly. I still don't understand why the version of typescript used in this repository affects what quicktype is doing, but it seems like the issue is in quicktype, not its dependency.

I validated this change was correct by diffing the output of `node scripts/generate-file-format-schema-json.js` with what's currently on http://speedscope.app/file-format-schema.json. There's no difference.

This PR also includes changes to the CI script to ensure that we can catch this before hitting master next time.
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.

Upgrading TypeScript broke JSON schema generation
2 participants