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

[Fix] correct generated type declaration #3840

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open
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
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);
ljharb marked this conversation as resolved.
Show resolved Hide resolved

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: Literal is not a valid type
(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 = {
ljharb marked this conversation as resolved.
Show resolved Hide resolved
'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
78 changes: 20 additions & 58 deletions lib/rules/jsx-no-literals.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,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<import('eslint').Rule.RuleModule['meta']['schema'], unknown[] | false>['properties']} */
const commonPropertiesSchema = {
noStrings: {
type: 'boolean',
Expand All @@ -65,56 +65,11 @@ 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
* @returns {ElementConfig}
* @param {import("../../types/rules/jsx-no-literals").RawConfig} config
* @returns {import("../../types/rules/jsx-no-literals").ElementConfig}
ljharb marked this conversation as resolved.
Show resolved Hide resolved
*/
function normalizeElementConfig(config) {
return {
Expand All @@ -128,13 +83,14 @@ function normalizeElementConfig(config) {
};
}

// eslint-disable-next-line valid-jsdoc
/**
* Normalizes the config and applies default values to all config options
* @param {RawConfig} config
* @returns {Config}
* @param {import("../../types/rules/jsx-no-literals").RawConfig} config
* @returns {import("../../types/rules/jsx-no-literals").Config}
*/
function normalizeConfig(config) {
/** @type {Config} */
/** @type {import("../../types/rules/jsx-no-literals").Config} */
const normalizedConfig = Object.assign(normalizeElementConfig(config), {
elementOverrides: {},
});
Expand Down Expand Up @@ -182,6 +138,7 @@ const elementOverrides = {
},
};

/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
meta: /** @type {import('eslint').Rule.RuleModule["meta"]} */ ({
docs: {
Expand All @@ -204,7 +161,7 @@ module.exports = {
}),

create(context) {
/** @type {RawConfig} */
/** @type {import("../../types/rules/jsx-no-literals").RawConfig} */
const rawConfig = (context.options.length && context.options[0]) || {};
const config = normalizeConfig(rawConfig);

Expand Down Expand Up @@ -339,11 +296,12 @@ 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
* @param {ASTNode} node
* @param {ResolvedConfig} resolvedConfig
* @param {import("../../types/rules/jsx-no-literals").ResolvedConfig} resolvedConfig
* @returns {boolean}
*/
function isViableTextNode(node, resolvedConfig) {
Expand All @@ -370,12 +328,13 @@ 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
* will also apply to the current node.
* @param {ASTNode} node
* @returns {OverrideConfig | undefined}
* @returns {import("../../types/rules/jsx-no-literals").OverrideConfig | undefined}
*/
function getOverrideConfig(node) {
if (!hasElementOverrides) {
Expand Down Expand Up @@ -408,17 +367,19 @@ module.exports = {
}
}

// eslint-disable-next-line valid-jsdoc
/**
* @param {ResolvedConfig} resolvedConfig
* @param {import("../../types/rules/jsx-no-literals").ResolvedConfig} resolvedConfig
* @returns {boolean}
*/
function shouldAllowElement(resolvedConfig) {
return resolvedConfig.type === 'override' && 'allowElement' in resolvedConfig && !!resolvedConfig.allowElement;
}

// eslint-disable-next-line valid-jsdoc
/**
* @param {boolean} ancestorIsJSXElement
* @param {ResolvedConfig} resolvedConfig
* @param {import("../../types/rules/jsx-no-literals").ResolvedConfig} resolvedConfig
* @returns {string}
*/
function defaultMessageId(ancestorIsJSXElement, resolvedConfig) {
Expand All @@ -433,10 +394,11 @@ module.exports = {
return resolvedConfig.type === 'override' ? 'literalNotInJSXExpressionInElement' : 'literalNotInJSXExpression';
}

// eslint-disable-next-line valid-jsdoc
/**
* @param {ASTNode} node
* @param {string} messageId
* @param {ResolvedConfig} resolvedConfig
* @param {import("../../types/rules/jsx-no-literals").ResolvedConfig} resolvedConfig
*/
function reportLiteralNode(node, messageId, resolvedConfig) {
report(context, messages[messageId], 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/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
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"
}
}
3 changes: 2 additions & 1 deletion test-published-types/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

"compilerOptions": {
"lib": ["esnext"],
"types": ["node"]
"types": ["node"],
"skipLibCheck": true
}
}
2 changes: 1 addition & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,5 @@
"alwaysStrict": false, /* Parse in strict mode and emit "use strict" for each source file. */
"resolveJsonModule": true
},
"include": ["lib"],
"include": ["lib", "types"],
}
Loading