Skip to content

Deprecate namespaces declared with module keyword #1

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

CC972
Copy link
Owner

@CC972 CC972 commented Feb 21, 2023

Fixes #

@CC972 CC972 marked this pull request as draft February 21, 2023 13:39
@CC972 CC972 force-pushed the deprecate-namespaces-declared-with-module-keyword branch from 265e593 to 92f6d4f Compare February 21, 2023 15:04
@CC972 CC972 changed the base branch from main to test February 21, 2023 15:12
@CC972 CC972 changed the base branch from test to main February 21, 2023 15:12
@acutmore
Copy link

I'm presuming we didn't want this to be configurable? Which I do agree with, doesn't seem worth adding an option to keep the existing behavior as this really should be 100% compatible with anyones current workflow unless they are somehow relying on this deprecated syntax.

Copy link

@acutmore acutmore left a comment

Choose a reason for hiding this comment

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

Nice work!

Co-authored-by: Ashley Claymore <[email protected]>
return node.modifiers !== modifiers
|| node.name !== name
|| node.body !== body
? update(createModuleDeclaration(modifiers, name, body, node.flags | NodeFlags.Namespace), node)
Copy link
Owner Author

Choose a reason for hiding this comment

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

As discussed with Titian, we should be able to replace this final return with simply:
return update(createModuleDeclaration(modifiers, name, body, node.flags | NodeFlags.Namespace), node)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants