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.

Sign up for GitHub

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

Jump to bottom

[Fix] @typescript-eslint v6 use typeArguments with fallback to typeParameters #3629

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
+70 −40
Draft

[Fix] @typescript-eslint v6 use typeArguments with fallback to typeParameters #3629

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
06bf6c3
[Fix] `function-component-definition`, `boolean-prop-naming`, `jsx-fi…
HenryBrown0 Sep 4, 2023
88674f6
[Tests] add @typescript-eslint/parser v6
HenryBrown0 Nov 27, 2023
1fcc3f4
fix: use @typescript-eslint/parser dist dir
HenryBrown0 Jan 16, 2024
5c0e8d0
refactor: add getTypeArguments util
HenryBrown0 Apr 26, 2024
358dfb8
refactor: tidy boolean prop naming type arg
HenryBrown0 Apr 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/node-18+.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ jobs:
with:
node-version: ${{ matrix.node-version }}
after_install: |
npm install --no-save "eslint@${{ matrix.eslint }}" "@typescript-eslint/parser@5" "babel-eslint@${{ matrix.babel-eslint }}"
npm install --no-save "eslint@${{ matrix.eslint }}" "@typescript-eslint/parser@6" "babel-eslint@${{ matrix.babel-eslint }}"
env:
NPM_CONFIG_LEGACY_PEER_DEPS: true
- run: npx ls-engines
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/node-minors.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ jobs:
with:
node-version: ${{ matrix.node-version }}
after_install: |
npm install --no-save "eslint@${{ matrix.eslint }}" "@typescript-eslint/parser@${{ matrix.node-version >= 14 && '5' || (matrix.node-version >= 12 && '4' || (matrix.node-version >= 10 && '4.0' || (matrix.node-version >= 8 && '3' || '2'))) }}" "babel-eslint@${{ matrix.babel-eslint }}"
npm install --no-save "eslint@${{ matrix.eslint }}" "@typescript-eslint/parser@${{ matrix.node-version >= 16 && '6' || matrix.node-version >= 14 && '5' || (matrix.node-version >= 12 && '4' || (matrix.node-version >= 10 && '4.0' || (matrix.node-version >= 8 && '3' || '2'))) }}" "babel-eslint@${{ matrix.babel-eslint }}"
skip-ls-check: ${{ matrix.node-version < 10 && true || false }}
env:
NPM_CONFIG_LEGACY_PEER_DEPS: true
Expand Down
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# Change Log

All notable changes to this project will be documented in this file.
This project adheres to [Semantic Versioning](https://semver.org/).
This change log adheres to standards from [Keep a CHANGELOG](https://keepachangelog.com).
Expand All @@ -8,8 +9,13 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
### Fixed
* [`boolean-prop-naming`]: avoid a crash with a non-TSTypeReference type ([#3718][] @developer-bandi)
* [`jsx-no-leaked-render`]: invalid report if left side is boolean ([#3746][] @akulsr0)
* [`function-component-definition`], [`boolean-prop-naming`], [`jsx-first-prop-new-line`], [`jsx-props-no-multi-spaces`], `propTypes`: use type args ([#3629][] @HenryBrown0)

### Changed
* [Tests] add @typescript-eslint/parser v6 ([#3629][] @HenryBrown0)

[#3746]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3746
[#3629]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3629
[#3718]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3718

## [7.34.1] - 2024.03.15
Expand Down
12 changes: 7 additions & 5 deletions lib/rules/boolean-prop-naming.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,14 +252,16 @@ module.exports = {
return;
}

const annotationTypeParams = component.node.parent.id.typeAnnotation.typeAnnotation.typeParameters;
const annotationTypeArguments = propsUtil.getTypeArguments(
component.node.parent.id.typeAnnotation.typeAnnotation
);
if (
annotationTypeParams && (
annotationTypeParams.type === 'TSTypeParameterInstantiation'
|| annotationTypeParams.type === 'TypeParameterInstantiation'
annotationTypeArguments && (
annotationTypeArguments.type === 'TSTypeParameterInstantiation'
|| annotationTypeArguments.type === 'TypeParameterInstantiation'
)
) {
return annotationTypeParams.params.find(
return annotationTypeArguments.params.find(
(param) => param.type === 'TSTypeReference' || param.type === 'GenericTypeAnnotation'
);
}
Expand Down
14 changes: 8 additions & 6 deletions lib/rules/function-component-definition.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const arrayIncludes = require('array-includes');
const Components = require('../util/Components');
const docsUrl = require('../util/docsUrl');
const reportC = require('../util/report');
const propsUtil = require('../util/props');

// ------------------------------------------------------------------------------
// Rule Definition
Expand All @@ -33,12 +34,12 @@ const UNNAMED_FUNCTION_TEMPLATES = {
};

function hasOneUnconstrainedTypeParam(node) {
const nodeTypeParams = node.typeParameters;
const nodeTypeArguments = propsUtil.getTypeArguments(node);

return nodeTypeParams
&& nodeTypeParams.params
&& nodeTypeParams.params.length === 1
&& !nodeTypeParams.params[0].constraint;
return nodeTypeArguments
&& nodeTypeArguments.params
&& nodeTypeArguments.params.length === 1
&& !nodeTypeArguments.params[0].constraint;
}

function hasName(node) {
Expand Down Expand Up @@ -202,11 +203,12 @@ module.exports = {
varType = node.parent.parent.kind;
}

const nodeTypeArguments = propsUtil.getTypeArguments(node);
return (fixer) => fixer.replaceTextRange(
options.range,
buildFunction(options.template, {
typeAnnotation,
typeParams: getNodeText(node.typeParameters, source),
typeParams: getNodeText(nodeTypeArguments, source),
params: getParams(node, source),
returnType: getNodeText(node.returnType, source),
body: getBody(node, source),
Expand Down
4 changes: 3 additions & 1 deletion lib/rules/jsx-first-prop-new-line.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

const docsUrl = require('../util/docsUrl');
const report = require('../util/report');
const propsUtil = require('../util/props');

// ------------------------------------------------------------------------------
// Rule Definition
Expand Down Expand Up @@ -55,7 +56,8 @@ module.exports = {
report(context, messages.propOnNewLine, 'propOnNewLine', {
node: decl,
fix(fixer) {
return fixer.replaceTextRange([(node.typeParameters || node.name).range[1], decl.range[0]], '\n');
const nodeTypeArguments = propsUtil.getTypeArguments(node);
return fixer.replaceTextRange([(nodeTypeArguments || node.name).range[1], decl.range[0]], '\n');
},
});
}
Expand Down
11 changes: 6 additions & 5 deletions lib/rules/jsx-props-no-multi-spaces.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

const docsUrl = require('../util/docsUrl');
const report = require('../util/report');
const propsUtil = require('../util/props');

// ------------------------------------------------------------------------------
// Rule Definition
Expand Down Expand Up @@ -99,26 +100,26 @@ module.exports = {
}

function containsGenericType(node) {
const nodeTypeParams = node.typeParameters;
if (typeof nodeTypeParams === 'undefined') {
const nodeTypeArguments = propsUtil.getTypeArguments(node);
if (typeof nodeTypeArguments === 'undefined') {
return false;
}

return nodeTypeParams.type === 'TSTypeParameterInstantiation';
return nodeTypeArguments.type === 'TSTypeParameterInstantiation';
}

function getGenericNode(node) {
const name = node.name;
if (containsGenericType(node)) {
const type = node.typeParameters;
const nodeTypeArguments = propsUtil.getTypeArguments(node);

return Object.assign(
{},
node,
{
range: [
name.range[0],
type.range[1],
nodeTypeArguments.range[1],
],
}
);
Expand Down
42 changes: 23 additions & 19 deletions lib/util/propTypes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -627,8 +627,8 @@ module.exports = function propTypesInstructions(context, components, utils) {
typeName = node.typeName.name;
const leftMostName = getLeftMostTypeName(node.typeName);
const shouldTraverseTypeParams = genericReactTypesImport.has(leftMostName);
const nodeTypeParams = node.typeParameters;
if (shouldTraverseTypeParams && nodeTypeParams && nodeTypeParams.length !== 0) {
const nodeTypeArguments = propsUtil.getTypeArguments(node);
if (shouldTraverseTypeParams && nodeTypeArguments && nodeTypeArguments.length !== 0) {
// All react Generic types are derived from:
// type PropsWithChildren<P> = P & { children?: ReactNode | undefined }
// So we should construct an optional children prop
Expand All @@ -639,7 +639,7 @@ module.exports = function propTypesInstructions(context, components, utils) {
const idx = genericTypeParamIndexWherePropsArePresent[
leftMostName !== rightMostName ? rightMostName : importedName
];
const nextNode = nodeTypeParams.params[idx];
const nextNode = nodeTypeArguments.params[idx];
this.visitTSNode(nextNode);
return;
}
Expand Down Expand Up @@ -728,10 +728,10 @@ module.exports = function propTypesInstructions(context, components, utils) {

convertReturnTypeToPropTypes(node) {
// ReturnType<T> should always have one parameter
const nodeTypeParams = node.typeParameters;
if (nodeTypeParams) {
if (nodeTypeParams.params.length === 1) {
let returnType = nodeTypeParams.params[0];
const nodeTypeArguments = propsUtil.getTypeArguments(node);
if (nodeTypeArguments) {
if (nodeTypeArguments.params.length === 1) {
let returnType = nodeTypeArguments.params[0];
// This line is trying to handle typescript-eslint-parser
// typescript-eslint-parser TSTypeQuery is wrapped by TSTypeReference
if (astUtil.isTSTypeReference(returnType)) {
Expand Down Expand Up @@ -763,9 +763,9 @@ module.exports = function propTypesInstructions(context, components, utils) {
case 'ObjectExpression':
iterateProperties(context, res.properties, (key, value, propNode) => {
if (propNode && propNode.argument && propNode.argument.type === 'CallExpression') {
const propNodeTypeParams = propNode.argument.typeParameters;
if (propNodeTypeParams) {
this.visitTSNode(propNodeTypeParams);
const propNodeTypeArguments = propsUtil.getTypeArguments(propNode.argument);
if (propNodeTypeArguments) {
this.visitTSNode(propNodeTypeArguments);
} else {
// Ignore this CallExpression return value since it doesn't have any typeParameters to let us know it's types.
this.shouldIgnorePropTypes = true;
Expand All @@ -785,8 +785,8 @@ module.exports = function propTypesInstructions(context, components, utils) {
});
break;
case 'CallExpression':
if (res.typeParameters) {
this.visitTSNode(res.typeParameters);
if (propsUtil.getTypeArguments(res)) {
this.visitTSNode(propsUtil.getTypeArguments(res));
} else {
// Ignore this CallExpression return value since it doesn't have any typeParameters to let us know it's types.
this.shouldIgnorePropTypes = true;
Expand Down Expand Up @@ -963,9 +963,9 @@ module.exports = function propTypesInstructions(context, components, utils) {
break;
case 'GenericTypeAnnotation':
if (propTypes.id.name === '$ReadOnly') {
const propTypeParams = propTypes.typeParameters;
const propTypeArguments = propsUtil.getTypeArguments(propTypes);
ignorePropsValidation = declarePropTypesForObjectTypeAnnotation(
propTypeParams.params[0],
propTypeArguments.params[0],
declaredPropTypes
);
} else {
Expand Down Expand Up @@ -1001,11 +1001,16 @@ module.exports = function propTypesInstructions(context, components, utils) {
return;
}

let propTypesArguments = null;
if (node.parent) {
propTypesArguments = propsUtil.getTypeArguments(node.parent);
}

if (
node.parent
&& node.parent.callee
&& node.parent.typeParameters
&& node.parent.typeParameters.params
&& propTypesArguments
&& propTypesArguments.params
&& (
node.parent.callee.name === 'forwardRef' || (
node.parent.callee.object
Expand All @@ -1015,9 +1020,8 @@ module.exports = function propTypesInstructions(context, components, utils) {
)
)
) {
const propTypesParams = node.parent.typeParameters;
const declaredPropTypes = {};
const obj = new DeclarePropTypesForTSTypeAnnotation(propTypesParams.params[1], declaredPropTypes);
const obj = new DeclarePropTypesForTSTypeAnnotation(propTypesArguments.params[1], declaredPropTypes);
components.set(node, {
declaredPropTypes: obj.declaredPropTypes,
ignorePropsValidation: obj.shouldIgnorePropTypes,
Expand Down Expand Up @@ -1063,7 +1067,7 @@ module.exports = function propTypesInstructions(context, components, utils) {
if (
annotation
&& annotation.type !== 'TSTypeReference'
&& annotation.typeParameters == null
&& propsUtil.getTypeArguments(annotation) == null
) {
return;
}
Expand Down
13 changes: 13 additions & 0 deletions lib/util/props.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,18 @@
return propTypeExpression.type === 'MemberExpression' && propTypeExpression.property.name === 'isRequired';
}

/**
* Returns the type arguments of a node or type parameters if type arguments are not available.
* @param {ASTNode} node The node to get the type arguments from.
* @returns {ASTNode} The type arguments or type parameters of the node.
*/
function getTypeArguments(node) {
if ('typeArguments' in node) {
return node.typeArguments;

Check warning on line 102 in lib/util/props.js

View check run for this annotation

Codecov / codecov/patch

lib/util/props.js#L102 

Added line #L102 was not covered by tests
}
return node.typeParameters;
}

module.exports = {
isPropTypesDeclaration,
isContextTypesDeclaration,
Expand All @@ -100,4 +112,5 @@
isDefaultPropsDeclaration,
isDisplayNameDeclaration,
isRequiredPropType,
getTypeArguments,
};
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
"@types/eslint": "=7.2.10",
"@types/estree": "0.0.52",
"@types/node": "^4.9.5",
"@typescript-eslint/parser": "^2.34.0 || ^3.10.1 || ^4.0.0 || ^5.0.0",
"@typescript-eslint/parser": "^2.34.0 || ^3.10.1 || ^4.0.0 || ^5.0.0 || ^6.0.0",
"aud": "^2.0.4",
"babel-eslint": "^8 || ^9 || ^10.1.0",
"eslint": "^3 || ^4 || ^5 || ^6 || ^7 || ^8",
Expand Down
2 changes: 1 addition & 1 deletion tests/helpers/parsers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ const parsers = {
BABEL_ESLINT: path.join(__dirname, NODE_MODULES, 'babel-eslint'),
'@BABEL_ESLINT': path.join(__dirname, NODE_MODULES, '@babel/eslint-parser'),
TYPESCRIPT_ESLINT: path.join(__dirname, NODE_MODULES, 'typescript-eslint-parser'),
'@TYPESCRIPT_ESLINT': path.join(__dirname, NODE_MODULES, '@typescript-eslint/parser'),
'@TYPESCRIPT_ESLINT': path.join(__dirname, NODE_MODULES, '@typescript-eslint/parser/dist'),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@typescript-eslint/parser removed the main field in v6, which pointed to dist/index.js. Not sure how tests were passing before, as this would have been wrong for all of v6.

Copy link
Member

Choose a reason for hiding this comment

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

exports['.'] would still work in node versions that support exports, which might be the only node versions that v6 supports?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

v6 supports node versions ^16.0.0 || >=18.0.0, but even if I'm running this locally with node 18 it still fails to find the export

Copy link
Member

Choose a reason for hiding this comment

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

interesting, indeed node 18 should work - line 15 in https://unpkg.com/browse/@typescript-eslint/[email protected]/package.json points there.

Copy link

Choose a reason for hiding this comment

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

It works* locally when I switch it to importing the specific file:

Suggested change
'@TYPESCRIPT_ESLINT': path.join(__dirname, NODE_MODULES, '@typescript-eslint/parser/dist'),
'@TYPESCRIPT_ESLINT': path.join(__dirname, NODE_MODULES, '@typescript-eslint/parser/dist/index.js'),

*as in, other things break later 🙃 - https://github.com/jsx-eslint/eslint-plugin-react/pull/3629/files#r1580983575

disableNewTS,
skipDueToMultiErrorSorting: semver.satisfies(process.versions.node, '^8 || ^9'),
babelParserOptions: function parserOptions(test, features) {
Expand Down