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

table-rename-isHoverable: Cannot read properties of undefined (reading 'name') #541

Open
Andrei-Stepanov opened this issue Oct 30, 2023 · 3 comments
Labels

Comments

@Andrei-Stepanov
Copy link

Hello,

Could you please help me?
I am following: https://www.patternfly.org/get-started/upgrade/release-notes/
I get an error:

➜  osci npx @patternfly/pf-codemods@latest ciboard
Need to install the following packages:
  @patternfly/[email protected]
Ok to proceed? (y) 
/home/astepano/.npm/_npx/d72ba669e7bba25a/node_modules/@patternfly/eslint-plugin-pf-codemods/lib/rules/v5/table-rename-isHoverable.js:10
    ).imports.filter((specifier) => ["Table", "Tr"].includes(specifier.imported.name));
                                                                                ^

TypeError: Error while loading rule '@patternfly/pf-codemods/table-rename-isHoverable': Cannot read properties of undefined (reading 'name')
Occurred while linting /home/astepano/osci/ciboard/src/components/SSTResultsTable.tsx
    at /home/astepano/.npm/_npx/d72ba669e7bba25a/node_modules/@patternfly/eslint-plugin-pf-codemods/lib/rules/v5/table-rename-isHoverable.js:10:81
    at Array.filter (<anonymous>)
    at Object.create (/home/astepano/.npm/_npx/d72ba669e7bba25a/node_modules/@patternfly/eslint-plugin-pf-codemods/lib/rules/v5/table-rename-isHoverable.js:10:15)
    at createRuleListeners (/home/astepano/.npm/_npx/d72ba669e7bba25a/node_modules/eslint/lib/linter/linter.js:910:21)
    at /home/astepano/.npm/_npx/d72ba669e7bba25a/node_modules/eslint/lib/linter/linter.js:1081:110
    at Array.forEach (<anonymous>)
    at runRules (/home/astepano/.npm/_npx/d72ba669e7bba25a/node_modules/eslint/lib/linter/linter.js:1018:34)
    at Linter._verifyWithoutProcessors (/home/astepano/.npm/_npx/d72ba669e7bba25a/node_modules/eslint/lib/linter/linter.js:1370:31)
    at Linter._verifyWithConfigArray (/home/astepano/.npm/_npx/d72ba669e7bba25a/node_modules/eslint/lib/linter/linter.js:1822:21)
    at Linter.verify (/home/astepano/.npm/_npx/d72ba669e7bba25a/node_modules/eslint/lib/linter/linter.js:1452:65)
node --version
v16.19.1


grep patter package.json 
    "@patternfly/react-charts": "^6.67.1",
    "@patternfly/react-core": "^4.276.6",
    "@patternfly/react-icons": "^4.65.1",
    "@patternfly/react-table": "^4.113.0",
@Andrei-Stepanov
Copy link
Author

Andrei-Stepanov commented Oct 30, 2023

Hi,
Update from a discussion on Slack with Dallas Nicol, it turned out that the place of error was:

import ReactTable from '@patternfly/react-table';

This commit fixed it:

fedora-ci/ciboard@7c869d2

@gitdallas
Copy link
Contributor

As far as fixing this on the codemod side. If we added a specifier?.name it would work for Andrei's case as the Table etc were imported by name.

However, it could be possible that we are wanting to target an import that could come through the default and used in that way (ReactTable.Table). In these cases if we just fixed with an undefined check, the codemod wouldn't be able to target them and do it's job (it would be ignored).

To really fix this, we'd have to make exceptions for this type of import... which would affect nearly all codemods and likely be a pretty significant effort.

@gitdallas
Copy link
Contributor

gitdallas commented Oct 30, 2023

As this is pretty uncommon and not a super trivial fix, I wonder if it would be good enough to ignore specifiers without import names (these default imports) by using the specifier.imported?.name (will have to check for all such instances in codemods) and then adding another codemod that just checks for such import specifiers and gives a warning like:

/src/path/file.tsx
10:1 warning This import specifier does not have an import name. The codemods will be unable to check against it. We recommend using named imports instead of default imports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Backlog
Development

No branches or pull requests

3 participants