GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — ignore static and declared member if checking override by Kingwl · Pull Request #43569 · microsoft/TypeScript

Conversation

@Kingwl

@Kingwl Wenlu Wang (Kingwl) commented Apr 7, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Fixes #43563
Fixes #43566

Declared property does not really override. So just ignore that.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@Kingwl
ignore static member if checking override
9ad18f2
@typescript-bot TypeScript Bot (typescript-bot) added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Apr 7, 2021
@Kingwl
Ignore declared member when check override
78d7b33
@Kingwl Wenlu Wang (Kingwl) changed the title ignore static member if checking override ignore static and declared member if checking override Apr 7, 2021
@andrewbranch Andrew Branch (andrewbranch) assigned Nathan Shively-Sanders (sandersn) and Andrew Branch (andrewbranch) Apr 7, 2021
@typescript-bot TypeScript Bot (typescript-bot) added For Milestone Bug PRs that fix a bug with a specific milestone and removed For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Apr 7, 2021
@andrewbranch

Andrew Branch (andrewbranch) commented Apr 7, 2021

Copy link
Copy Markdown
Member

I think I would expect that under --noImplicitOverride, overriding a static property with another static property would require the override keyword. The issue with #43563 is that it didn’t seem to recognize that static and instance properties are distinct at all. But maybe I’m missing something?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
andrewbranch
Andrew Branch (andrewbranch) requested changes Apr 7, 2021
View reviewed changes

@andrewbranch Andrew Branch (andrewbranch) left a comment

Copy link
Copy Markdown
Member

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.

Discussed in the design meeting and decided that yes, static overrides need the override keyword in --noImplicitOverride.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) mentioned this pull request Apr 7, 2021
Design Meeting Notes, 4/7/2021 #43583
Closed
@Kingwl
Check static override too
5992394
sandersn
Nathan Shively-Sanders (sandersn) approved these changes Apr 8, 2021
View reviewed changes
andrewbranch
Andrew Branch (andrewbranch) reviewed Apr 8, 2021
View reviewed changes
Comment thread tests/cases/conformance/override/override13.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Wenlu Wang (Kingwl) added 2 commits April 9, 2021 11:09
@Kingwl
Merge branch 'master' into fix_override_issues
6e7f30c
@Kingwl
Add more tests
9453f68
andrewbranch
Andrew Branch (andrewbranch) approved these changes Apr 9, 2021
View reviewed changes

@andrewbranch Andrew Branch (andrewbranch) left a comment

Copy link
Copy Markdown
Member

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 Wenlu Wang (@Kingwl)!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@andrewbranch
Andrew Branch (andrewbranch) merged commit b0c2860 into microsoft:master Apr 9, 2021
@Kingwl Wenlu Wang (Kingwl) mentioned this pull request May 1, 2021
problem with override and static fields in ts4.3 beta #43916
Closed
@Kingwl
Wenlu Wang (Kingwl) deleted the fix_override_issues branch May 6, 2021 04:01
@microsoft Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@sandersn Nathan Shively-Sanders (sandersn) sandersn approved these changes
@andrewbranch Andrew Branch (andrewbranch) andrewbranch approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@sandersn Nathan Shively-Sanders (sandersn)

@andrewbranch Andrew Branch (andrewbranch)

Labels

For Milestone Bug PRs that fix a bug with a specific milestone

Projects

PR Backlog
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Property using declare prompts to add 'override' even though that does not seem valid noImplicitOverride complains about static property not overriding non-static property

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@Kingwl @andrewbranch @sandersn @typescript-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.