Skip to content

fix(37578): Deprecate variable name AllowQualifedNameInPlaceOfIdentifier #38726

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

Merged
merged 4 commits into from
Feb 17, 2021
Merged

fix(37578): Deprecate variable name AllowQualifedNameInPlaceOfIdentifier #38726

merged 4 commits into from
Feb 17, 2021

Conversation

cljazouli
Copy link
Contributor

Description:
Change variable name AllowQualifedNameInPlaceOfIdentifier to AllowQualifiedNameInPlaceOfIdentifier

Bug introduced in: #14709

Expected behavior:
AllowQualifedNameInPlaceOfIdentifier should be deprecated in favor of AllowQualifiedNameInPlaceOfIdentifier

Actual behavior:
AllowQualifedNameInPlaceOfIdentifier is not deprecated

Fixes #37578

Copy link

@Maxim-Mazurok Maxim-Mazurok left a comment

Choose a reason for hiding this comment

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

In #37578 I asked to deprecate misspelled variable instead of removing it, as it may break backward-compatibility (in linters, etc.)

It was previously done here, for example:

/** @deprecated Use ProjectReferenceCycle_OutputsSkipped instead. */
ProjectReferenceCycle_OutputsSkupped = 4,

@@ -3691,7 +3691,7 @@ namespace ts {

// Error handling
AllowThisInObjectLiteral = 1 << 15,
AllowQualifedNameInPlaceOfIdentifier = 1 << 16,
AllowQualifiedNameInPlaceOfIdentifier = 1 << 16,

Choose a reason for hiding this comment

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

In #37578 I asked to deprecate misspelled variable instead of removing it, as it may break backward-compatibility (in linters, etc.)

It was previously done here, for example:

/** @deprecated Use ProjectReferenceCycle_OutputsSkipped instead. */
ProjectReferenceCycle_OutputsSkupped = 4,

What do you think about it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yeah, that sounds much better than deleting it. I just updated my code to deprecate it instead.

Copy link

@Maxim-Mazurok Maxim-Mazurok left a comment

Choose a reason for hiding this comment

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

LGTM!

@cljazouli cljazouli changed the title Change variable name AllowQualifedNameInPlaceOfIdentifier to AllowQua… fix(37578): Deprecate variable name AllowQualifedNameInPlaceOfIdentifier May 29, 2020
@cljazouli
Copy link
Contributor Author

cc @RyanCavanaugh

Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

Thanks for this fix. I requested one minor change; if you no longer have time for it, let me know and I'll try to get to it myself.

@@ -5366,6 +5366,7 @@ namespace ts {

if (expectsIdentifier && chain.length !== 1
&& !context.encounteredError
&& !(context.flags & NodeBuilderFlags.AllowQualifiedNameInPlaceOfIdentifier)
&& !(context.flags & NodeBuilderFlags.AllowQualifedNameInPlaceOfIdentifier)) {
Copy link
Member

Choose a reason for hiding this comment

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

the new value is the same as the old, so you should replace the old usage instead of adding to it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

Couple more things I noticed

@@ -3702,7 +3704,7 @@ namespace ts {
AllowNodeModulesRelativePaths = 1 << 26,
/* @internal */ DoNotIncludeSymbolChain = 1 << 27, // Skip looking up and printing an accessible symbol chain

IgnoreErrors = AllowThisInObjectLiteral | AllowQualifedNameInPlaceOfIdentifier | AllowAnonymousIdentifier | AllowEmptyUnionOrIntersection | AllowEmptyTuple | AllowEmptyIndexInfoType | AllowNodeModulesRelativePaths,
IgnoreErrors = AllowThisInObjectLiteral | AllowQualifedNameInPlaceOfIdentifier | AllowQualifiedNameInPlaceOfIdentifier | AllowAnonymousIdentifier | AllowEmptyUnionOrIntersection | AllowEmptyTuple | AllowEmptyIndexInfoType | AllowNodeModulesRelativePaths,
Copy link
Member

Choose a reason for hiding this comment

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

same here -- replace the old

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@@ -3691,6 +3691,8 @@ namespace ts {

// Error handling
AllowThisInObjectLiteral = 1 << 15,
AllowQualifiedNameInPlaceOfIdentifier = 1 << 16,
/** @deprecated AllowQualifedNameInPlaceOfIdentifier. Use AllowQualifiedNameInPlaceOfIdentifier instead. */
AllowQualifedNameInPlaceOfIdentifier = 1 << 16,
Copy link
Member

Choose a reason for hiding this comment

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

please replace 1 << 16 with AllowQualifiedNameInPlaceOfIdentifier

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@cljazouli
Copy link
Contributor Author

@sandersn Thanks! I will make those changes

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Sep 4, 2020
@cljazouli cljazouli requested a review from sandersn September 4, 2020 15:53
Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

I'll merge this after 4.1 RC gets its own branch, so that it'll ship in 4.1.

@sandersn sandersn self-assigned this Oct 27, 2020
@sandersn sandersn merged commit 3fe05c8 into microsoft:master Feb 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Typo in AllowQualifedNameInPlaceOfIdentifier
4 participants