FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(io/buffer): super and initialized prop by kitsonk · Pull Request #1917 · denoland/std · GitHub

/ std Public

fix(io/buffer): super and initialized prop - #1917

Merged
kitsonk merged 1 commit into
denoland:mainfrom
kitsonk:super_first
Feb 14, 2022
ConversationCommits (1)ChecksFiles changed
Merged

fix(io/buffer): super and initialized prop#1917
kitsonk merged 1 commit into
denoland:mainfrom
kitsonk:super_first

Conversation

kitsonk commented Feb 14, 2022

Copy link
Copy Markdown
Contributor

This gets rid of the error messages like the following when using io/buffer.ts under tsc directly via dnt:

TS2376: A 'super' call must be the first statement in the constructor when a class contains initialized properties, parameter properties, or private identifiers.

While this is being fixed in future version of TypeScript, it is a limitation that is easy to work around and allows people to use the std lib via dnt without type errors.

kitsonk requested a review from dsherret February 14, 2022 03:22
kitsonk mentioned this pull request Feb 14, 2022
Closed
kt3k approved these changes Feb 14, 2022
View reviewed changes

kt3k left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

I was actually seeing the same error while using dnt

kt3k commented Feb 14, 2022

Copy link
Copy Markdown
Contributor

@kitsonk I'm in favor of this change, but I'm not sure why this is ok in std and causes type error when using dnt transformation. Can this be somehow tested in std's CI?

kitsonk commented Feb 14, 2022

Copy link
Copy Markdown
Contributor Author

@kt3k no, because it only occurs when using tsc directly and not targeting esnext. The fix is in TypeScript 4.6, and this is the PR that fixed it: microsoft/TypeScript#29374. You cannot force Deno to typecheck in non-esnext mode. The only way to manifest this would be to run some sort of process the converts everything via dnt to "node" code and then runs tsc against it, and I see that seperate to this PR if such a thing is desired.

kitsonk mentioned this pull request Feb 14, 2022
Closed

kitsonk commented Feb 14, 2022

Copy link
Copy Markdown
Contributor Author

I

@kitsonk I'm in favor of this change, but I'm not sure why this is ok in std and causes type error when using dnt transformation. Can this be somehow tested in std's CI?

I opened #1918 to cover this.

kitsonk merged commit a9f6a1c into denoland:main Feb 14, 2022
kitsonk deleted the super_first branch February 14, 2022 04:03

kt3k commented Feb 14, 2022

Copy link
Copy Markdown
Contributor

Thanks for the explanation and creating the issue!

microsoft/TypeScript#29374.

Oh, that birthday cake PR!

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

bartlomieju Awaiting requested review from bartlomieju
dsherret Awaiting requested review from dsherret
1 more reviewer
kt3k kt3k approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

Back | FazBrowse Home | New Git URL