Skip to content
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

[eslint-plugin] feat: no-deprecated-popover2-components rule #6474

Merged
merged 3 commits into from
Oct 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 43 additions & 24 deletions packages/eslint-plugin/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ Note that this rule only supports hardcoded values in the `icon` prop; it does n

A fixer is available for this rule that will convert between string literals and named `Icon` components. Note that the implementation is naive and may require intervention, such as to import a component or fix an invalid name.

Named icon components (`TickIcon`, `GraphIcon`, etc) can be imported from the `@blueprintjs/icons` package.
Named icon components (`Tick`, `Graph`, etc) can be imported from the `@blueprintjs/icons` package.

This rule is disabled in the `blueprint-rules` config as it is most useful to ensure that the `@blueprintjs/icons` package can be tree-shaken (an opt-in process which requires using components and _never_ `IconName` literals).

Expand Down Expand Up @@ -137,32 +137,41 @@ This rule is disabled in the `blueprint-rules` config as it is most useful to en

### `@blueprintjs/no-deprecated-components`

Ban usage of deprecated Blueprint components, including:
Ban usage of deprecated components in the current major version of all Blueprint packages, including:

- AbstractComponent
- AbstractPureComponent
- Breadcrumbs
- CollapsibleList
- Breadcrumbs2
- ContextMenu2
- DateInput
- DateInput2
- DatePicker
- DateRangeInput
- DateTimePicker
- MenuItem
- MultiSelect
- PanelStack
- Popover
- Select
- Suggest
- TimezonePicker
- Tooltip

**Rationale**: Many Blueprint components have "V2" variants as a part of their natural API evolution.
Blueprint consumers are recommended to migrate from the deprecated V1 components to their newer V2 counterparts
in order to future-proof their code for the next major version of Blueprint, where the V2 components will become the
only available API and the V1 variants will be removed. Flagging usage of deprecated APIs can be done with other
ESLint rules like `deprecation/deprecation`, but that rule is often too broad to enable as an "error" globally across
a large code base. `@blueprintjs/no-deprecated-components` provides a simpler, more scoped rule which only flags
usage of deprecated Blueprint components in JSX. The idea here is that you can enable this rule as an "error" in ESLint
to prevent any backwards progress in your Blueprint migration as you move from V1 -> V2 APIs in preparation for v5.0.
- DateRangeInput2
- DateRangePicker
- MenuItem2
- Popover2
- ResizeSensor2
- Tooltip2

**Rationale**: There are two reasons why a particular component may be deprecated:

1. Many Blueprint components have next-generation variants as a part of their natural API evolution. Blueprint
consumers are recommended to migrate from the deprecated "V1" components to their newer V2 counterparts
in order to future-proof their code for the next major version of Blueprint, where the V2 components will become
the only available API and the "V1" variants will be removed. Flagging usage of deprecated APIs can be done with
other ESLint rules like `deprecation/deprecation`, but that rule is often too broad to enable as an "error"
globally across a large code base. `@blueprintjs/no-deprecated-components` provides a simpler, more scoped rule
which only flags usage of deprecated Blueprint components in JSX syntax. The idea here is that you can enable this
rule as an "error" in ESLint to prevent any backwards progress in your Blueprint migration as you migrate to newer
APIs in preparation for the next major version.

2. When a next-generation component API (designated as "V2" or "V3") is "promoted" to the standard "V1" API in a
major version of Blueprint, we keep around aliases for the "V2" names which referred to that same API in the
_previous_ major version. These names are immediately marked as `/** @deprecated */` and you are encouraged to
migrate to their corresponding "V1" symbols after a major upgrade. For example,
`{ Popover2 } from "@blueprintjs/popover2"` is the recommended popover API in Blueprint v4.x, but it is deprecated
in Blueprint v5.x. If you continue to use `Popover2` in v5.x, you will get a lint error suggesting a rename
migration to `{ Popover } from "@blueprintjs/core"`. Since these symbols are aliases for each other in v5.x, there
should be no runtime impact from this kind of migration.

### `@blueprintjs/no-deprecated-core-components`

Expand Down Expand Up @@ -194,6 +203,16 @@ Similar to `@blueprintjs/no-deprecated-components`, but only flags usage of depr
deprecated `@blueprintjs/datetime2` component usage as errors while allowing deprecated components from other packages
to pass as lint warnings.

### `@blueprintjs/no-deprecated-popover2-components`

Similar to `@blueprintjs/no-deprecated-components`, but only flags usage of deprecated components from the
`@blueprintjs/popover2` package instead of all `@blueprintjs/` packages.

**Rationale**: In migrations of large code bases, it may be useful to apply more granular rule configuration of
"no-deprecated-components" to make incremental progress towards the newer APIs. This allows you, for example, to flag
deprecated `@blueprintjs/popover2` component usage as errors while allowing deprecated components from other packages
to pass as lint warnings.

### `@blueprintjs/no-deprecated-select-components`

Similar to `@blueprintjs/no-deprecated-components`, but only flags usage of deprecated components from the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,25 @@ import { TSESLint, TSESTree } from "@typescript-eslint/utils";

import { createRule } from "../utils/createRule";

type MessageIds = "migration" | "migrationWithPropUsage";
type MessageIds =
| "migration"
| "migrationToNewPackage"
| "migrationWithPropUsage"
| "migrationWithPropUsageToNewPackage";

/**
* Key is either:
* - the name of a deprecated component, or
* - a component name + prop identifier (e.g. "ComponentV1.propName") which indicates that usage of
* <ComponentV1 propName={...}> should be flagged.
*
* Value is either:
* - the name of a corresponding non-deprecated component (if it exists in the same package), or
* - a tuple containing:
* 1. name of non-deprecated component
* 2. the package where that component is found
*/
export type DeprecatedComponentsConfig = Record<string, string | [string, string]>;

/**
* Higher-order function to create an ESLint rule which checks for usage of deprecated React components in JSX syntax.
Expand All @@ -17,17 +35,12 @@ type MessageIds = "migration" | "migrationWithPropUsage";
*
* @param deprecatedComponentConfig Configuration of the deprecated components to lint for. Note that this configuration
* is not exposed to lint rule users, it just lives inside our rule implementations. Lint violations will include a
* recommendation to migrate to the newer, non-deprecated component specified in this mapping. Keys-value pairs may use
* one of two syntaxes:
* - "ComponentV1": "ComponentV2" - Usage of <ComponentV1> will be flagged with a recommendation
* to migrate to <ComponentV2>
* - "ComponentV1.propName": "ComponentV2" - Usage of <ComponentV1 propName={...}> will be flagged with a
* recommendation to migrate to <ComponentV2>
* recommendation to migrate to the newer, non-deprecated component specified in this mapping.
*/
export function createNoDeprecatedComponentsRule(
ruleName: string,
packagesToCheck: string[],
deprecatedComponentConfig: Record<string, string>,
deprecatedComponentConfig: DeprecatedComponentsConfig,
): TSESLint.RuleModule<MessageIds, unknown[]> {
const descriptionFromClause = packagesToCheck.length === 1 ? ` from ${packagesToCheck[0]}` : "";

Expand All @@ -43,8 +56,12 @@ export function createNoDeprecatedComponentsRule(
messages: {
migration:
"Usage of {{ deprecatedComponentName }} is deprecated, migrate to {{ newComponentName }} instead",
migrationToNewPackage:
"Usage of {{ deprecatedComponentName }} is deprecated, migrate to {{ newComponentName }} from '{{ newPackageName }}' instead",
migrationWithPropUsage:
"Usage of {{ deprecatedComponentName }} with prop '{{ deprecatedPropName }}' is deprecated, migrate to {{ newComponentName }} instead",
migrationWithPropUsageToNewPackage:
"Usage of {{ deprecatedComponentName }} with prop '{{ deprecatedPropName }}' is deprecated, migrate to {{ newComponentName }} from '{{ newPackageName }}' instead",
},
schema: [
{
Expand Down Expand Up @@ -108,20 +125,17 @@ export function createNoDeprecatedComponentsRule(
return;
}

const deprecatedComponentKey = Object.keys(deprecatedComponentConfig).find(
key => key.includes(".") && key.split(".")[0] === elementName,
);
const deprecatedPropName = ((deprecatedProp as TSESTree.JSXAttribute).name as TSESTree.JSXIdentifier)
.name;
context.report({
data: {
deprecatedComponentName: elementName,

context.report(
getReportDescriptor(
jsxOpeningElementChildNode,
deprecatedComponentConfig,
elementName,
deprecatedPropName,
newComponentName: deprecatedComponentConfig[deprecatedComponentKey!],
},
messageId: "migrationWithPropUsage",
node: jsxOpeningElementChildNode,
});
),
);
}

// Get the list of all deprecated imports from packages included in the provided list
Expand Down Expand Up @@ -158,14 +172,7 @@ export function createNoDeprecatedComponentsRule(
// check <DeprecatedComponent> syntax (includes self-closing tags)
"JSXElement > JSXOpeningElement > JSXIdentifier": (node: TSESTree.JSXIdentifier) => {
if (isDeprecatedComponent(node.name)) {
context.report({
data: {
deprecatedComponentName: node.name,
newComponentName: deprecatedComponentConfig[node.name],
},
messageId: "migration",
node,
});
context.report(getReportDescriptor(node, deprecatedComponentConfig, node.name));
} else if (isOpeningElement(node.parent)) {
// check <DeprecatedComponent withDeprecatedProp={...}> syntax
checkDeprecatedComponentAndProp(node, node.name, node.parent);
Expand All @@ -186,14 +193,9 @@ export function createNoDeprecatedComponentsRule(
const namespaceIdentifier = node.object;
if (isDeprecatedNamespacedComponent(namespaceIdentifier.name, node.property.name)) {
// Uses a blueprint namespace and a deprecated property
context.report({
data: {
deprecatedComponentName: node.property.name,
newComponentName: deprecatedComponentConfig[node.property.name],
},
messageId: "migration",
node: node.property,
});
context.report(
getReportDescriptor(node.property, deprecatedComponentConfig, node.property.name),
);
} else if (isOpeningElement(node.parent)) {
// check <Blueprint.DeprecatedComponent withDeprecatedProp={...}> syntax
checkDeprecatedComponentAndProp(node, node.property.name, node.parent);
Expand All @@ -204,14 +206,7 @@ export function createNoDeprecatedComponentsRule(
"ClassDeclaration[superClass.type='Identifier']": (node: TSESTree.ClassDeclaration) => {
const superClass = node.superClass as TSESTree.Identifier;
if (isDeprecatedComponent(superClass.name)) {
context.report({
data: {
deprecatedComponentName: superClass.name,
newComponentName: deprecatedComponentConfig[superClass.name],
},
messageId: "migration",
node,
});
context.report(getReportDescriptor(node, deprecatedComponentConfig, superClass.name));
}
},

Expand All @@ -228,14 +223,13 @@ export function createNoDeprecatedComponentsRule(
const namespaceIdentifier = superClass.object;
if (isDeprecatedNamespacedComponent(namespaceIdentifier.name, superClass.property.name)) {
// Uses a blueprint namespace and a deprecated property
context.report({
data: {
deprecatedComponentName: superClass.property.name,
newComponentName: deprecatedComponentConfig[superClass.property.name],
},
messageId: "migration",
node: superClass.property,
});
context.report(
getReportDescriptor(
superClass.property,
deprecatedComponentConfig,
superClass.property.name,
),
);
}
},

Expand All @@ -246,15 +240,7 @@ export function createNoDeprecatedComponentsRule(
}

if (isDeprecatedComponent(node.object.name)) {
const deprecatedComponentName = node.object.name;
context.report({
data: {
deprecatedComponentName,
newComponentName: deprecatedComponentConfig[deprecatedComponentName],
},
messageId: "migration",
node: node.object,
});
context.report(getReportDescriptor(node.object, deprecatedComponentConfig, node.object.name));
}
},
};
Expand All @@ -265,3 +251,39 @@ export function createNoDeprecatedComponentsRule(
function isOpeningElement(parent: TSESTree.Node | undefined): parent is TSESTree.JSXOpeningElement {
return parent?.type === TSESTree.AST_NODE_TYPES.JSXOpeningElement;
}

/**
* Returns a lint report descriptor for a particular kind of deprecated component usage.
* Parses the config to determine the appropriate message ID and data to include in the report.
*/
function getReportDescriptor(
node: TSESTree.Node,
config: DeprecatedComponentsConfig,
deprecatedComponentName: string,
deprecatedPropName?: string,
): TSESLint.ReportDescriptor<MessageIds> {
const configKey =
deprecatedPropName === undefined ? deprecatedComponentName : `${deprecatedComponentName}.${deprecatedPropName}`;
const configValue = config[configKey];
const [newComponentName, newPackageName] = Array.isArray(configValue) ? configValue : [configValue, undefined];

const messageId =
deprecatedPropName === undefined
? newPackageName === undefined
? "migration"
: "migrationToNewPackage"
: newPackageName === undefined
? "migrationWithPropUsage"
: "migrationWithPropUsageToNewPackage";

return {
data: {
deprecatedComponentName,
deprecatedPropName,
newComponentName,
newPackageName,
},
messageId,
node,
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,6 @@ export * from "./no-deprecated-components";
export * from "./no-deprecated-core-components";
export * from "./no-deprecated-datetime-components";
export * from "./no-deprecated-datetime2-components";
export * from "./no-deprecated-popover2-components";
export * from "./no-deprecated-select-components";
export * from "./no-deprecated-table-components";
Original file line number Diff line number Diff line change
Expand Up @@ -8,28 +8,33 @@ import { createNoDeprecatedComponentsRule } from "./createNoDeprecatedComponents
import { coreComponentsMigrationMapping } from "./no-deprecated-core-components";
import { datetimeComponentsMigrationMapping } from "./no-deprecated-datetime-components";
import { datetime2ComponentsMigrationMapping } from "./no-deprecated-datetime2-components";
import { popover2ComponentsMigrationMapping } from "./no-deprecated-popover2-components";
import { selectComponentsMigrationMapping } from "./no-deprecated-select-components";
import { tableComponentsMigrationMapping } from "./no-deprecated-table-components";

/**
* This rule checks a hardcoded list of components that Blueprint is actively migrating to a newer version (e.g. v1 -> v2)
* If deprecated versions (v1) are detected, it will recommend using the replacement component (e.g. the v2) instead.
* Note that this does not rely on the \@deprecated JSDoc annotation, and is thus distinct/very different from the
* deprecated/deprecated ESLint rule
* This rule checks target source code against a static list of deprecated React components.
* If deprecated versions are detected, it will recommend using the replacement component instead
* (for example, "v1" API -> "v2" API).
*
* Note that this implementation does not rely on the \@deprecated JSDoc annotation, and is thus distinct
* from the 'deprecated/deprecated' ESLint rule.
*/
export const noDeprecatedComponentsRule: TSESLint.RuleModule<string, unknown[]> = createNoDeprecatedComponentsRule(
"no-deprecated-components",
[
"@blueprintjs/core",
"@blueprintjs/datetime",
"@blueprintjs/datetime2",
"@blueprintjs/popover2",
"@blueprintjs/select",
"@blueprintjs/table",
],
{
...coreComponentsMigrationMapping,
...datetimeComponentsMigrationMapping,
...datetime2ComponentsMigrationMapping,
...popover2ComponentsMigrationMapping,
...selectComponentsMigrationMapping,
...tableComponentsMigrationMapping,
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* (c) Copyright 2023 Palantir Technologies Inc. All rights reserved.
*/

import type { TSESLint } from "@typescript-eslint/utils";

import { createNoDeprecatedComponentsRule, type DeprecatedComponentsConfig } from "./createNoDeprecatedComponentsRule";

export const popover2ComponentsMigrationMapping: DeprecatedComponentsConfig = {
Breadcrumbs2: ["Breadcrumbs", "@blueprintjs/core"],
ContextMenu2: ["ContextMenu", "@blueprintjs/core"],
MenuItem2: ["MenuItem", "@blueprintjs/core"],
Popover2: ["Popover", "@blueprintjs/core"],
ResizeSensor2: ["ResizeSensor", "@blueprintjs/core"],
Tooltip2: ["Tooltip", "@blueprintjs/core"],
};

/**
* This rule is similar to "@blueprintjs/no-deprecated-components", but it only checks for usage
* of deprecated components from @blueprintjs/popover2. This is useful for incremental migration to
* newer Blueprint APIs.
*/
export const noDeprecatedPopover2ComponentsRule: TSESLint.RuleModule<string, unknown[]> =
createNoDeprecatedComponentsRule(
"no-deprecated-popover2-components",
["@blueprintjs/popover2"],
popover2ComponentsMigrationMapping,
);
Loading