-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Fix 4628: add compiler option for non-check static member in inheritance #39699
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 4628: add compiler option for non-check static member in inheritance #39699
Conversation
I think a keyword like
Why I would like to have a keyword like this? |
It's been long enough that we should discuss this in a design meeting or backlog slog again. @DanielRosenwasser Also, the original issue said that we should try running with this turned on to see how many errors disappear. What happens when you do that? |
Hello @sandersn + @ShuiRuTian, I believe this PR would solve the problem I bumped into. In order to manage my expectations, do you know what a reasonable ETA would be for this PR to get merged and released ? Thanks ! |
It is not clear to me why the error is on the class and not on the static method. I would like to disable this error... but I have other static methods in the same class that would benefit from this type of type check. I would like to be able to disable it at the static method level instead of the class level... or the project level... but anything will be appreciated since it will fix a 5 year old issue (#4628) PlaygroundLink to a playground simplified version of the problem... That I describe in my issue #4628 comment
|
I would keep an eye on this PR #39669. I think it is a better and more natural choice to fix this issue. PS: I know that this PR and the linked PR is not the same thing. This PR is to allow static method could be override with different function signature, the linked one is make override behavior must have "override" keyword. |
Any chance we can revive this PR? This would solve a lot of problems for us. Happy to contribute if there is a chance we can bring this PR to a merge |
@richardgirges We never got a clear picture of when errors disappear, or how common those contexts are. We really need some examples to decide whether this is a good idea. You can look through the following test results, although it may not have enough examples of usage to help: @typescript-bot user test this line |
Argh (not 'line') |
@typescript-bot user test this inline |
Heya @sandersn, I've started to run the diff-based community code test suite on this PR at 2badcb3. You can monitor the build here. Update: The results are in! |
@sandersn Here they are:Comparison Report - main..refs/pull/39699/merge [adonis-framework]1 of 1 projects failed to build with the old tsc /mnt/ts_downloads/adonis-framework/tsconfig.json
|
@typescript-bot user test this inline |
Heya @sandersn, I've started to run the diff-based community code test suite on this PR at 2badcb3. You can monitor the build here. Update: The results are in! |
@sandersn Here they are:Comparison Report - main..refs/pull/39699/merge [adonis-framework]1 of 1 projects failed to build with the old tsc /mnt/ts_downloads/adonis-framework/tsconfig.json
[assert]1 of 1 projects failed to build with the old tsc /mnt/ts_downloads/assert/tsconfig.json
[async]1 of 1 projects failed to build with the old tsc /mnt/ts_downloads/async/tsconfig.json
[bcryptjs]1 of 1 projects failed to build with the old tsc /mnt/ts_downloads/bcryptjs/tsconfig.json
[bluebird]1 of 1 projects failed to build with the old tsc /mnt/ts_downloads/bluebird/tsconfig.json
[debug]1 of 1 projects failed to build with the old tsc /mnt/ts_downloads/debug/tsconfig.json
[discord.js]/mnt/ts_downloads/discord.js/tsconfig.json
fp-ts2 of 4 projects failed to build with the old tsc /mnt/ts_downloads/fp-ts/dtslint/ts3.5/tsconfig.json
[graceful-fs]1 of 1 projects failed to build with the old tsc /mnt/ts_downloads/graceful-fs/tsconfig.json
[lodash]1 of 1 projects failed to build with the old tsc /mnt/ts_downloads/lodash/tsconfig.json
[rxjs]/mnt/ts_downloads/rxjs/tsconfig.json
[uglify-js]1 of 1 projects failed to build with the old tsc /mnt/ts_downloads/uglify-js/tsconfig.json
[url-search-params]1 of 1 projects failed to build with the old tsc /mnt/ts_downloads/url-search-params/tsconfig.json
|
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.
Go
This PR has been stale for a long time now, and I never got a good idea of what this changes and therefore whether it's a good idea. It really needs more design work, so I'm going to close the PR. |
Fixes #4628