Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

fix(noRedeclare): allow merging namespace + variable #4772

Merged
merged 2 commits into from
Aug 19, 2023

Conversation

llllvvuu
Copy link
Contributor

@llllvvuu llllvvuu commented Aug 19, 2023

fixes #4771

Note: tsc doesn't always allow merging namespaces and variables. Either way, checking this shouldn't fall under the linter - either it's a type error or it's a declaration merge, but it's never a shadow.

@netlify
Copy link

netlify bot commented Aug 19, 2023

Deploy Preview for docs-rometools canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit d405ef6
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/64e0a66f9c4b200007cbdf06

@github-actions github-actions bot added the A-Parser Area: parser label Aug 19, 2023
@llllvvuu llllvvuu changed the title test: failing namespace + variable case for noRedeclare fix(noRedeclare): allow merging namespace + variable Aug 19, 2023
@llllvvuu llllvvuu marked this pull request as ready for review August 19, 2023 09:05
@llllvvuu llllvvuu marked this pull request as draft August 19, 2023 09:20
@llllvvuu llllvvuu closed this Aug 19, 2023
@llllvvuu llllvvuu reopened this Aug 19, 2023
@llllvvuu llllvvuu marked this pull request as ready for review August 19, 2023 09:30
@llllvvuu llllvvuu marked this pull request as draft August 19, 2023 09:35
@llllvvuu llllvvuu marked this pull request as ready for review August 19, 2023 09:40
@Conaclos
Copy link
Contributor

Note: tsc doesn't always allow merging namespaces and variables. Either

I was not aware of the merging capability between a variable and an only-type namespace.
For now the fix looks fair.

@llllvvuu
Copy link
Contributor Author

llllvvuu commented Aug 19, 2023

Yes, TypeScript is very weird 😂 Concrete variable example added!

CI failure should be fixed by #4766

@Conaclos Conaclos self-requested a review August 19, 2023 18:24
Copy link
Contributor

@Conaclos Conaclos left a comment

Choose a reason for hiding this comment

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

Thanks!

@Conaclos Conaclos merged commit 9f3bbe9 into rome:main Aug 19, 2023
18 of 19 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Linter Area: linter A-Parser Area: parser L-JavaScript Langauge: JavaScript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 bug(lint/noRedeclare): false positive for namespace + variable declaration merging
2 participants