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

feat: add support for unknown in typescript #2194

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

RichiCoder1
Copy link

Adds support for unknown (and it's flow equivalent) in TypeScript.

First PR and I made this constribution mostly via the GitHub UI, happy to do more if needed!

Closes #1619

Adds support for `unknown` (and it's flow equivalent) in TypeScript.

Closes glideapps#1619
@dvdsgl
Copy link
Member

dvdsgl commented Mar 1, 2023

Awesome! Please add an option for this in the typescript test fixture.

@RichiCoder1
Copy link
Author

@dvdsgl absolutely! Pulled down source to attempt this, but I'm not sure where in tests I'd add the specific options?

Also ran into this issue while executing npm run test:

[INFO] Compiling 3 source files to /workspaces/quicktype/test/fixtures/java/target/classes
[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
[ERROR] Source option 6 is no longer supported. Use 7 or later.
[ERROR] Target option 6 is no longer supported. Use 7 or later.
[INFO] 2 errors 
[INFO] -------------------------------------------------------------
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  4.015 s
[INFO] Finished at: 2023-03-01T20:56:22Z
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.10.1:compile (default-compile) on project QuickTypeTest: Compilation failure: Compilation failure: 
[ERROR] Source option 6 is no longer supported. Use 7 or later.
[ERROR] Target option 6 is no longer supported. Use 7 or later.
[ERROR] -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException

@dvdsgl
Copy link
Member

dvdsgl commented Mar 1, 2023

Check the README for details. You want quicktestRenderSettings or some such name.

@RichiCoder1
Copy link
Author

@RichiCoder1
Copy link
Author

Yup that was it. Actually found an unrelated bug, but icing this PR for a second. The source uses "any" as a catchall in a number of places where a more specific type (Record<key, unknown>, IConvertable) would make more sense which means a decent size chunk-o-changes will likely need to be made to the JavaScript output to emit the correct and specific types.

@dvdsgl dvdsgl marked this pull request as draft June 3, 2023 23:32
@SBrandeis
Copy link

@RichiCoder1 are there any plans to move forward with this PR?

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.

add flag for typescript to use unknown instead of any
3 participants