-
Notifications
You must be signed in to change notification settings - Fork 98
Update tsconfig and corresponding docs #1488
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
Conversation
Our generated blueprint was *years* out of date, and between the changes we made in the types over the last year or so and the guidance we have given in the TS RFCs we are now both *able* and *required* to set `strict: true` as well as to opt into `noUncheckedIndexedAccess` and to disable `allowSyntheticDefaultImports` and `esModuleInterop`. Additionally, this provided a nice opportunity to document the choices we have made so folks have something to latch onto when looking at the config we generate for them, and to update the config to target the current latest versions of the `target` and `module` fields.
6e0a767 to
3271c7b
Compare
NullVoxPopuli
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding all the context/comments! <3
dfreeman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉 Thank you!
| "declaration": true, | ||
| "declarationMap": true, | ||
| "inlineSourceMap": true, | ||
| "inlineSources": true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that we set noEmit: true here and use Babel for actually emitting code, do these flags have any effect?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't actually know! These are what I added four years ago. 😬
| "allowJs": true, | ||
| "target": "ES2022", | ||
| "module": "ES2022", | ||
| "moduleResolution": "node", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not actionable yet, but I suspect we're going to want to change this to node12 as soon as that graduates from nightly, since Webpack 5 (and so by extension, ember-auto-import and Embroider) honor exports config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep. Hoping they ship that in 4.7 and we can flip it on. 🤞🏼
Co-authored-by: Dan Freeman <[email protected]>
Our generated blueprint was years out of date, and between the changes we made in the types over the last year or so and the guidance we have given in the TS RFCs we are now both able and required to set
strict: trueas well as to opt intonoUncheckedIndexedAccessand to disableallowSyntheticDefaultImportsandesModuleInterop.Additionally, this provided a nice opportunity to document the choices we have made so folks have something to latch onto when looking at the config we generate for them, and to update the config to target the current latest versions of the
targetandmodulefields.