prefer-wait-for: Cannot read property 'arguments' of null #272
Comments
|
Hi AriPerkkio , thanks for posting. How would the test usage look like? Just a regular waitForElement(() => getByText('foo')) ? I wouldn't recommend storing the queries in the global space. You can always import Either way, we could try to fix this scenario to prevent the suite from crashing |
|
I should have mentioned that the use case causing the crash might not be valid or recommended. I'm just testing stability of various community ESLint plugins and reporting the issues to plugin developers. This same issue could come up in completely valid case too. This is also crashing: import { waitForElement } from '@testing-library/react';
const renamedWaitForElement = waitForElement; |
|
Hello Ari. Thanks for opening this issue. You are right: this scenario shouldn't cause any error. However, we are close to finish v4 of the plugin, which includes a huge refactor of all rules almost from the ground. I can confirm this case has been fixed on v4 as you can see here: |
|
Thanks for the verification @Belco90. Feel free to close this issue. Once v4 is available in npm I'll include it to my weekly scheduled CI runs on eslint-remote-tester. |
|
Awesome! I think I'm gonna leave it open as I did with other issues solved on v4 until it gets released. |
|
This should be fixed on v4.0.0 The release is available on: |
|
Hey @AriPerkkio! I saw after releasing v4 of the plugin the badge for this ESLint plugin is green in your Would be fine to include such a badge in our README? |
|
Congratulations about v4! 🎉 I was excited to see how it stands against heavy 6 hour tests - and it was perfect score with 0 crashes. Here are some comparisons from other plugins. The results are not 100% comparable but it provides some useful feedback about plugin's performance.
Definitely! If you are interested in keeping |
|
@AriPerkkio Would it also be possible to have your workflows report automatically in our repo somehow? 🤔 You're already running all tests, so CI would run a second time on this repo for no other reason than to report possible bugs to us. Can you also provide us with the list of the other ESLint plugins that already use your action themselves please? |
I'm not sure whether Github provides functionality for this. The workflow is currently utilizing octokit client for creating the issues. I'll need to look into octokit's documentation - PRs to
Note that So far |
@AriPerkkio Since this package is using Running these smoke tests in this repo will also result in a 6 hour run for each push to We would also have to update/maintain the |
|
It would be interesting to run this check to have some insights about the plugin, but we shouldn't block any package release because of this check. |
|
That's why I think it would be better to have this run in the original Having this as a bot would be the way to go I think, as that way we can give/revoke permissions. |
|
Good points @MichaelDeBoey. Let's keep running the tests in I would imagine something like |
|
Thanks for your help @AriPerkkio! I'll include the badge in the following days. Probably we can move the reporting from |
|
@AriPerkkio I forgot to mention: another possibility would be running |
|
That's definitely possible and even better option than waiting for my weekly scheduled tests to trigger. You wouldn't have to wait 7 days for getting the latest results. There is of course this:
... but it is not really that much of a work. I haven't updated my
Also consider the case where you make a breaking change or add new rule. This change would have to be made into But as said both options are possible, it is up to you to decide which way to go. |
|
@AriPerkkio We can start with the weekly triggered tests on your repo for now and maybe manually putting all errors in an issue here I think. We can discuss this maybe even further in an issue/discussion on your repo so we don't hijack this issue any further? |
Hello,
prefer-wait-forrule seems to crash in certain cases. This issue was spotted by automated CI run - it is not blocking my development or anything. ESlint rules should not crash in any condition since this makes all valid linting problems disappear. If this is a false flag please let me know.https://github.com/AriPerkkio/eslint-remote-tester/runs/1551964310?check_suite_focus=true
Complete list of dependencies and
.eslintrcis available at CI runs logs, stepsRun yarn list | grep eslintandRun yarn log --config ./plugin-configs/eslint-plugin-testing-library.config.js.Minimal repro:
Crash reports from real projects
Rule: prefer-wait-for
Cannot read property 'arguments' of null Occurred while linting <text>:2diondirza/rexr/test/test-utils.jsRule: prefer-wait-for
Cannot read property 'arguments' of null Occurred while linting <text>:18gravitational/webapps/packages/design/src/utils/testing.tsxRule: prefer-wait-for
Cannot read property 'arguments' of null Occurred while linting <text>:2faultable/wardex/src/setupTests.jsRule: prefer-wait-for
Cannot read property 'arguments' of null Occurred while linting <text>:2activewidgets/react/test/adapter/react.jsRule: prefer-wait-for
Cannot read property 'arguments' of null Occurred while linting <text>:3w-sr/React-Storybook-Game/src/setupTests.jsRule: prefer-wait-for
Cannot read property 'arguments' of null Occurred while linting <text>:20sumup-oss/circuit-ui/src/util/test-utils.tsxThe text was updated successfully, but these errors were encountered: