Enable checking Typescript files behind a compiler flag #4753
Conversation
|
Hi @drewatk! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
...lay-compiler/tests/relay_compiler_integration/fixtures/preloadable_query_typescript.expected
Outdated
Show resolved
Hide resolved
.../relay-compiler/tests/relay_compiler_integration/fixtures/preloadable_query_typescript.input
Outdated
Show resolved
Hide resolved
| disable_edge_type_name_validation_on_declerative_connection_directives: Default::default(), | ||
| typescript_disable_checking_generated_files: default_true(), | ||
| } | ||
| } |
There was a problem hiding this comment.
I think it's okay if we skip this for now and rely on the derived default. I think it's okay to let all places that use default feature flags from Default::default() in Rust instead of via serde (just tests I think?) use false as the default here.
I assume that's what you started with. Was there some reason it was problematic to let those tests default to false?
Summary: A fix for a typescript generation bug discovered from the work in #4753, with efforts to enable checking for all TS generated files #4745 This fixes a class of Typescript errors in generated typescript files with TS checking enabled: ``` /src/__generated__/mutation.graphql.ts(11,27): error TS2305: Module '"relay-runtime"' has no exported member 'Mutation'. /src/__generated__/mutation.graphql.ts(11,27): error TS6133: 'Mutation' is declared but its value is never read. ``` The root cause is the exported type is both not exported by the TS types, and not used due to typescript using the `export default` syntax. Where flow would generate the following export: ``` module.exports = ((node/*: any*/)/*: Mutation< validateMutationTest9ChangeNameIncludeMutation$variables, validateMutationTest9ChangeNameIncludeMutation$data, >*/); ``` Typescript would generate the following, not using the `Mutation` type: ``` export default node; ``` This fix removes the extraneous import type when the language is set as typescript. Pull Request resolved: #4754 Differential Revision: D60604034 fbshipit-source-id: f38fc80bba3b549a22ebbca544dca1f9e940cce0
|
Hey @captbaritone, let me know what you think about this approach and if there are any blockers to merging this. I've updated it to the latest code and checks are passing |
Fix for #4745
Behind a compiler flag,
typescript_check_generated_files, removes the@ts-nocheckflag from being output within typescript generated files by the Relay Compiler.This is not safe to enable yet, but will allow for identifying Typescript issues in the generated code by enabling this flag to find those issues.
Notes: