-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
Allow some per file compiler options #49886
base: main
Are you sure you want to change the base?
Conversation
@typescript-bot pack this |
Heya @weswigham, I've started to run the tarball bundle task on this PR at 459196b. You can monitor the build here. |
@typescript-bot pack this now that baselines should be green again :) |
Heya @weswigham, I've started to run the tarball bundle task on this PR at 97bba40. You can monitor the build here. |
Is this opt-in only, or will it be possible to opt-out as well? |
You should be able to write things like |
@typescript-bot pack this |
Heya @weswigham, I've started to run the tarball bundle task on this PR at ef3813c. You can monitor the build here. |
Hey @weswigham, I've packed this into an installable tgz. You can install it for testing by referencing it in your
and then running There is also a playground for this build and an npm module you can use via |
WIP 2 Less WIP - Only exactOptionalPropertyTypes and strictNullChecks left Pragmas are case-insensitive Fix harness types Fixup baselines File-local support for exactOptionalPropertyTypes Fix lints Cache getSourceFileOfNode within the checker
ef3813c
to
b45165d
Compare
...nes/reference/genericConditionalConstrainedToUnknownNotAssignableToConcreteObject.errors.txt
Outdated
Show resolved
Hide resolved
1be381d
to
fe5600e
Compare
This should be cleaned up and ready to go, so let's get the bot to do some tests on this iteration (not the much has changed implementation-wise since I ran them on the strict null checks-dedicated draft pr). @typescript-bot pack this |
Heya @weswigham, I've started to run the extended test suite on this PR at 4bde2e7. You can monitor the build here. |
Heya @weswigham, I've started to run the perf test suite on this PR at 4bde2e7. You can monitor the build here. Update: The results are in! |
@typescript-bot perf test this faster |
Heya @weswigham, I've started to run the abridged perf test suite on this PR at 638a3bf. You can monitor the build here. Update: The results are in! |
@weswigham Here they are:Comparison Report - main..49886
System
Hosts
Scenarios
Developer Information: |
@typescript-bot perf test this faster |
Heya @weswigham, I've started to run the abridged perf test suite on this PR at 5b77328. You can monitor the build here. Update: The results are in! |
@weswigham Here they are:Comparison Report - main..49886
System
Hosts
Scenarios
Developer Information: |
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.
lgtm
Hi, any updates on this? Now that it is approved, can we expect it to be merged anytime soon? |
@typescript-bot pack this |
Hey @weswigham, if you don't mind, can you please ask the @typescript-bot to pack this latest iteration? It seems to listen to the PR author. I wanted to try it out in some local project. Thanks! |
@typescript-bot pack this |
Heya @jakebailey, I've started to run the tarball bundle task on this PR at 5b77328. You can monitor the build here. |
Hey @jakebailey, I've packed this into an installable tgz. You can install it for testing by referencing it in your
and then running There is also a playground for this build and an npm module you can use via |
Thanks @jakebailey! |
There is only a week before ts 5.0 beta. Maybe this feature will be avaliable after 5.1 |
I'm seeing these kinds of errors despite turning off the check. Is this expected? Ideally it should not be imo as turning off SNC from tsconfig would not generate this error. |
I was writing up a much longer background for this, but I’m exhausted on Saturday, and don’t want to lose the context before I bail. I’m happy to come back and fill in details of why I’m asking if it seems really bonkers to ask. Is there any possibility this could allow specifying:
The use case prompting me to ask involves using Playwright as a library, and would incredibly simplify its usage (and type safety) for a single function proxy as I transition a library from dependence on Node APIs and native dependencies to be usable in browser. It’s definitely not a common use case, but I can imagine others which might benefit from specifying a given callback is being shuttled off to be executed in an entirely different environment. |
Hey all, the future of this feature is somewhat uncertain. Any time the type system has to make a decision based on strictness, it now has to walk up to the top of the file and check for strictness directives. If not that, every node (or almost every node) of our syntax trees has to get a little bit bigger to hold onto that information. This has a type-checking performance impact that is higher than we can accept. There might be some optimizations we can make for the more common case of no file-specific options within a project, but that leads to some unpredictable performance cliffs as soon as one is added. So this won't make its way into TypeScript 5.0, and it's unclear if we'll be able to bring it in in the future either. |
Can this option be not added as an opt in? like an option in I came across this issue searching for a way to enable incremental strict checks on a 50k+ line codebase on which enabling strict checks throws up 3k+ errors 🙈 while i understand plugins exist for this having this on a compiler level can be so much better! I know no nothing about typescripts development process/priorities so if anything i said here is wrong/doesnt make sense I apologise but I sincerely hope you consider the request! Thank you to the author and typescript team for their time and effort on this! |
In this PR, we allow certain compiler options to be configured via comments within a file for the scope of that file using a comment of the form
// @ts-option [value]
. This means you can write, for example:Currently this supports everything in the "strict type checks" and "additional checks" segments of options, which includes:
strict
noImplicitAny
strictNullChecks
strictFunctionTypes
strictBindCallApply
strictPropertyInitialization
noImplicitThis
useUnknownInCatchVariables
alwaysStrict
noUnusedLocals
noUnusedParameters
exactOptionalPropertyTypes
noImplicitReturns
noFallthroughCasesInSwitch
noUncheckedIndexedAccess
noImplicitOverride
noPropertyAccessFromIndexSignature
Adding more options to this list is just a matter of adding the
allowedAsPragma
flag to the definition of the option incommandLineParser.ts
and usinggetFileLocalCompilerOption
to look up the option at relevant locations, so we can easily add support for more local options as needed (by far the hardest wasstrictNullChecks
).Fixes #31035
I've wanted to look into this since I originally added the generic pragma-parsing infrastructure for
@ts-check
and related comments.To start off the discussion, in the linked issue, @RyanCavanaugh asked:
Currently in this PR, whichever is stricter. If either file the types originated from has it set, it's related strictly. Could go the other way and relate loosely if either is explicitly non-strict if looser checking is more preferable. Honestly, most of these "which context controls the flag" already have fairly canonical answers, either because we already look up a context to check JS-y-ness (for JS-file-specific-behaviors or error ignoring), or to issue an error in (in which case the error node location is a pretty logical context).
Anyways, I'm hoping this implementation can jumpstart discussion on the value of in-file pragmas for compiler options, specifically with the goal of bikeshedding the syntax and what options we should ultimately expose on a per-file basis. (Technically, I've tried to make the internal APIs for adding new per-file pragmas very type-safe and very easy to use.)
I strongly recommend reviewing this by-commit if possible. Specifically, the changes to
strictNullChecks
to support this are extensive and are contained in f5134a5.