Skip to content

Commit

Permalink
[Fix] types: correct generated type declaration
Browse files Browse the repository at this point in the history
- Add types/rules/jsx-no-literals.d.ts to avoid error TS2309: An export assignment cannot be used in a module with other exported elements.
  • Loading branch information
ocavue authored and ljharb committed Oct 12, 2024
1 parent 4ef92b4 commit e80def4
Show file tree
Hide file tree
Showing 20 changed files with 144 additions and 89 deletions.
20 changes: 18 additions & 2 deletions .github/workflows/type-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,34 @@ jobs:
- name: build types
run: npm run build-types

# Pack the lib into a tarball so that when we install the lib later in the
# test-published-types directory, it's only install `dependencies` of the
# lib.
- name: pack the lib
run: npm pack --pack-destination /tmp/

- name: find the packed lib
run: echo "ESLINT_PLUGIN_REACT_PATH=$(ls /tmp/eslint-plugin-react*.tgz | tail -n 1)" >> $GITHUB_ENV

- name: show the path to the packed lib
run: echo "$ESLINT_PLUGIN_REACT_PATH"

- name: npm install working directory
run: npm install
working-directory: test-published-types

- name: install typescript version ${{ matrix.ts_version }}
run: npm install --no-save typescript@${{ matrix.ts_version }}
- name: install eslint-plugin-react and typescript version ${{ matrix.ts_version }}
run: npm install --no-save "$ESLINT_PLUGIN_REACT_PATH" typescript@${{ matrix.ts_version }}
working-directory: test-published-types

- name: show installed typescript version
run: npm list typescript --depth=0
working-directory: test-published-types

- name: show installed eslint-plugin-react version
run: npm list eslint-plugin-react --depth=0
working-directory: test-published-types

- name: check types with lib "${{ matrix.ts_lib }}"
run: npx tsc --lib ${{ matrix.ts_lib }}
working-directory: test-published-types
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange

### Fixed
* [`no-danger`]: avoid a crash on a nested component name ([#3833][] @ljharb)
* [Fix] types: correct generated type declaration ([#3840][] @ocavue)

### Changed
* [Tests] [`jsx-no-script-url`]: Improve tests ([#3849][] @radu2147)

[#3849]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3849
[#3840]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3840
[#3833]: https://github.com/jsx-eslint/eslint-plugin-react/issues/3833

## [7.37.2] - 2024.10.22
Expand Down
54 changes: 29 additions & 25 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ function filterRules(rules, predicate) {

/**
* @param {object} rules - rules object mapping rule name to rule module
* @returns {Record<string, 2 | 'error'>}
* @returns {Record<string, SEVERITY_ERROR | 'error'>}
*/
function configureAsError(rules) {
return fromEntries(Object.keys(rules).map((key) => [`react/${key}`, 2]));
Expand All @@ -31,6 +31,10 @@ const plugins = [
'react',
];

// TODO: with TS 4.5+, inline this
const SEVERITY_ERROR = /** @type {2} */ (2);
const SEVERITY_OFF = /** @type {0} */ (0);

const configs = {
recommended: {
plugins,
Expand All @@ -40,28 +44,28 @@ const configs = {
},
},
rules: {
'react/display-name': 2,
'react/jsx-key': 2,
'react/jsx-no-comment-textnodes': 2,
'react/jsx-no-duplicate-props': 2,
'react/jsx-no-target-blank': 2,
'react/jsx-no-undef': 2,
'react/jsx-uses-react': 2,
'react/jsx-uses-vars': 2,
'react/no-children-prop': 2,
'react/no-danger-with-children': 2,
'react/no-deprecated': 2,
'react/no-direct-mutation-state': 2,
'react/no-find-dom-node': 2,
'react/no-is-mounted': 2,
'react/no-render-return-value': 2,
'react/no-string-refs': 2,
'react/no-unescaped-entities': 2,
'react/no-unknown-property': 2,
'react/no-unsafe': 0,
'react/prop-types': 2,
'react/react-in-jsx-scope': 2,
'react/require-render-return': 2,
'react/display-name': SEVERITY_ERROR,
'react/jsx-key': SEVERITY_ERROR,
'react/jsx-no-comment-textnodes': SEVERITY_ERROR,
'react/jsx-no-duplicate-props': SEVERITY_ERROR,
'react/jsx-no-target-blank': SEVERITY_ERROR,
'react/jsx-no-undef': SEVERITY_ERROR,
'react/jsx-uses-react': SEVERITY_ERROR,
'react/jsx-uses-vars': SEVERITY_ERROR,
'react/no-children-prop': SEVERITY_ERROR,
'react/no-danger-with-children': SEVERITY_ERROR,
'react/no-deprecated': SEVERITY_ERROR,
'react/no-direct-mutation-state': SEVERITY_ERROR,
'react/no-find-dom-node': SEVERITY_ERROR,
'react/no-is-mounted': SEVERITY_ERROR,
'react/no-render-return-value': SEVERITY_ERROR,
'react/no-string-refs': SEVERITY_ERROR,
'react/no-unescaped-entities': SEVERITY_ERROR,
'react/no-unknown-property': SEVERITY_ERROR,
'react/no-unsafe': SEVERITY_OFF,
'react/prop-types': SEVERITY_ERROR,
'react/react-in-jsx-scope': SEVERITY_ERROR,
'react/require-render-return': SEVERITY_ERROR,
},
},
all: {
Expand All @@ -82,8 +86,8 @@ const configs = {
jsxPragma: null, // for @typescript/eslint-parser
},
rules: {
'react/react-in-jsx-scope': 0,
'react/jsx-uses-react': 0,
'react/react-in-jsx-scope': SEVERITY_OFF,
'react/jsx-uses-react': SEVERITY_OFF,
},
},
};
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/forbid-foreign-prop-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ module.exports = {
&& !ast.isAssignmentLHS(node)
&& !isAllowedAssignment(node)
)) || (
// @ts-expect-error The JSXText type is not present in the estree type definitions
// @ts-expect-error: The JSXText type is not present in the estree type definitions
(node.property.type === 'Literal' || node.property.type === 'JSXText')
&& 'value' in node.property
&& node.property.value === 'propTypes'
Expand Down
1 change: 1 addition & 0 deletions lib/rules/forbid-prop-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ module.exports = {
const propTypesSpecifier = node.specifiers.find((specifier) => (
'imported' in specifier
&& specifier.imported
&& 'name' in specifier.imported
&& specifier.imported.name === 'PropTypes'
));
if (propTypesSpecifier) {
Expand Down
1 change: 1 addition & 0 deletions lib/rules/forward-ref-uses-ref.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ const messages = {
removeForwardRef: 'Remove forwardRef wrapper',
};

/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
meta: {
docs: {
Expand Down
4 changes: 3 additions & 1 deletion lib/rules/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
/* eslint global-require: 0 */

/** @satisfies {Record<string, import('eslint').Rule.RuleModule>} */
module.exports = {
const rules = {
'boolean-prop-naming': require('./boolean-prop-naming'),
'button-has-type': require('./button-has-type'),
'checked-requires-onchange-or-readonly': require('./checked-requires-onchange-or-readonly'),
Expand Down Expand Up @@ -108,3 +108,5 @@ module.exports = {
'style-prop-object': require('./style-prop-object'),
'void-dom-elements-no-children': require('./void-dom-elements-no-children'),
};

module.exports = rules;
7 changes: 6 additions & 1 deletion lib/rules/jsx-fragments.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,12 @@ module.exports = {
ImportDeclaration(node) {
if (node.source && node.source.value === 'react') {
node.specifiers.forEach((spec) => {
if ('imported' in spec && spec.imported && spec.imported.name === fragmentPragma) {
if (
'imported' in spec
&& spec.imported
&& 'name' in spec.imported
&& spec.imported.name === fragmentPragma
) {
if (spec.local) {
fragmentNames.add(spec.local.name);
}
Expand Down
66 changes: 18 additions & 48 deletions lib/rules/jsx-no-literals.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@ const docsUrl = require('../util/docsUrl');
const report = require('../util/report');
const getText = require('../util/eslint').getText;

/** @typedef {import('eslint').Rule.RuleModule} RuleModule */

/** @typedef {import('../../types/rules/jsx-no-literals').Config} Config */
/** @typedef {import('../../types/rules/jsx-no-literals').RawConfig} RawConfig */
/** @typedef {import('../../types/rules/jsx-no-literals').ResolvedConfig} ResolvedConfig */
/** @typedef {import('../../types/rules/jsx-no-literals').OverrideConfig} OverrideConfig */
/** @typedef {import('../../types/rules/jsx-no-literals').ElementConfig} ElementConfig */

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------
Expand Down Expand Up @@ -45,7 +53,7 @@ const messages = {
literalNotInJSXExpressionInElement: 'Missing JSX expression container around literal string: "{{text}}" in {{element}}',
};

/** @type {Exclude<import('eslint').Rule.RuleModule['meta']['schema'], unknown[]>['properties']} */
/** @type {Exclude<RuleModule['meta']['schema'], unknown[] | false>['properties']} */
const commonPropertiesSchema = {
noStrings: {
type: 'boolean',
Expand All @@ -65,52 +73,7 @@ const commonPropertiesSchema = {
},
};

/**
* @typedef RawElementConfigProperties
* @property {boolean} [noStrings]
* @property {string[]} [allowedStrings]
* @property {boolean} [ignoreProps]
* @property {boolean} [noAttributeStrings]
*
* @typedef RawOverrideConfigProperties
* @property {boolean} [allowElement]
* @property {boolean} [applyToNestedElements=true]
*
* @typedef {RawElementConfigProperties} RawElementConfig
* @typedef {RawElementConfigProperties & RawElementConfigProperties} RawOverrideConfig
*
* @typedef RawElementOverrides
* @property {Record<string, RawOverrideConfig>} [elementOverrides]
*
* @typedef {RawElementConfig & RawElementOverrides} RawConfig
*
* ----------------------------------------------------------------------
*
* @typedef ElementConfigType
* @property {'element'} type
*
* @typedef ElementConfigProperties
* @property {boolean} noStrings
* @property {Set<string>} allowedStrings
* @property {boolean} ignoreProps
* @property {boolean} noAttributeStrings
*
* @typedef OverrideConfigProperties
* @property {'override'} type
* @property {string} name
* @property {boolean} allowElement
* @property {boolean} applyToNestedElements
*
* @typedef {ElementConfigType & ElementConfigProperties} ElementConfig
* @typedef {OverrideConfigProperties & ElementConfigProperties} OverrideConfig
*
* @typedef ElementOverrides
* @property {Record<string, OverrideConfig>} elementOverrides
*
* @typedef {ElementConfig & ElementOverrides} Config
* @typedef {Config | OverrideConfig} ResolvedConfig
*/

// eslint-disable-next-line valid-jsdoc
/**
* Normalizes the element portion of the config
* @param {RawConfig} config
Expand All @@ -128,6 +91,7 @@ function normalizeElementConfig(config) {
};
}

// eslint-disable-next-line valid-jsdoc
/**
* Normalizes the config and applies default values to all config options
* @param {RawConfig} config
Expand Down Expand Up @@ -182,8 +146,9 @@ const elementOverrides = {
},
};

/** @type {RuleModule} */
module.exports = {
meta: /** @type {import('eslint').Rule.RuleModule["meta"]} */ ({
meta: /** @type {RuleModule['meta']} */ ({
docs: {
description: 'Disallow usage of string literals in JSX',
category: 'Stylistic Issues',
Expand Down Expand Up @@ -339,6 +304,7 @@ module.exports = {
return some(iterFrom([ancestors.parent, ancestors.grandParent]), (parent) => jsxElementTypes.has(parent.type));
}

// eslint-disable-next-line valid-jsdoc
/**
* Determines whether a given node's value and its immediate parent are
* viable text nodes that can/should be reported on
Expand Down Expand Up @@ -370,6 +336,7 @@ module.exports = {
return isStandardJSXNode && parent.type !== 'JSXExpressionContainer';
}

// eslint-disable-next-line valid-jsdoc
/**
* Gets an override config for a given node. For any given node, we also
* need to traverse the ancestor tree to determine if an ancestor's config
Expand Down Expand Up @@ -408,6 +375,7 @@ module.exports = {
}
}

// eslint-disable-next-line valid-jsdoc
/**
* @param {ResolvedConfig} resolvedConfig
* @returns {boolean}
Expand All @@ -416,6 +384,7 @@ module.exports = {
return resolvedConfig.type === 'override' && 'allowElement' in resolvedConfig && !!resolvedConfig.allowElement;
}

// eslint-disable-next-line valid-jsdoc
/**
* @param {boolean} ancestorIsJSXElement
* @param {ResolvedConfig} resolvedConfig
Expand All @@ -433,6 +402,7 @@ module.exports = {
return resolvedConfig.type === 'override' ? 'literalNotInJSXExpressionInElement' : 'literalNotInJSXExpression';
}

// eslint-disable-next-line valid-jsdoc
/**
* @param {ASTNode} node
* @param {string} messageId
Expand Down
1 change: 1 addition & 0 deletions lib/rules/jsx-props-no-spread-multi.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const messages = {
noMultiSpreading: 'Spreading the same expression multiple times is forbidden',
};

/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
meta: {
docs: {
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/jsx-space-before-closing.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ module.exports = {
const sourceCode = getSourceCode(context);

const leftToken = getTokenBeforeClosingBracket(node);
const closingSlash = /** @type {import("eslint").AST.Token} */ (sourceCode.getTokenAfter(leftToken));
const closingSlash = /** @type {import('eslint').AST.Token} */ (sourceCode.getTokenAfter(leftToken));

if (leftToken.loc.end.line !== closingSlash.loc.start.line) {
return;
Expand Down
4 changes: 2 additions & 2 deletions lib/rules/no-access-state-in-setstate.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ module.exports = {
&& node.object.type === 'ThisExpression'
&& isClassComponent(node)
) {
/** @type {import("eslint").Rule.Node} */
/** @type {import('eslint').Rule.Node} */
let current = node;
while (current.type !== 'Program') {
// Reporting if this.state is directly within this.setState
Expand Down Expand Up @@ -163,7 +163,7 @@ module.exports = {

Identifier(node) {
// Checks if the identifier is a variable within an object
/** @type {import("eslint").Rule.Node} */
/** @type {import('eslint').Rule.Node} */
let current = node;
while (current.parent.type === 'BinaryExpression') {
current = current.parent;
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/no-deprecated.js
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ module.exports = {
}
node.specifiers.filter(((s) => 'imported' in s && s.imported)).forEach((specifier) => {
// TODO, semver-major: remove `in` check as part of jsdoc->tsdoc migration
checkDeprecation(node, 'imported' in specifier && `${MODULES[node.source.value][0]}.${specifier.imported.name}`, specifier);
checkDeprecation(node, 'imported' in specifier && 'name' in specifier.imported && `${MODULES[node.source.value][0]}.${specifier.imported.name}`, specifier);
});
},

Expand Down
2 changes: 1 addition & 1 deletion lib/rules/no-unused-state.js
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ module.exports = {
&& unwrappedRight.type === 'ObjectExpression'
) {
// Find the nearest function expression containing this assignment.
/** @type {import("eslint").Rule.Node} */
/** @type {import('eslint').Rule.Node} */
let fn = node;
while (fn.type !== 'FunctionExpression' && fn.parent) {
fn = fn.parent;
Expand Down
4 changes: 3 additions & 1 deletion test-published-types/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@
const react = require('eslint-plugin-react');

/** @type {import('eslint').Linter.Config[]} */
module.exports = [
const config = [
{
plugins: {
react,
},
},
];

module.exports = config;
3 changes: 1 addition & 2 deletions test-published-types/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
"private": true,
"version": "0.0.0",
"dependencies": {
"eslint": "^9.11.1",
"eslint-plugin-react": "file:.."
"eslint": "^9.11.1"
}
}
Loading

0 comments on commit e80def4

Please sign in to comment.