Closed mitchell-merry closed 4 months ago
Not really sure why the build is suddenly failing, maybe you know:
Run actions/setup-node@v2
Found in cache @ /opt/hostedtoolcache/node/16.20.2/x64
/usr/local/bin/yarn --version
1.22.21
/usr/local/bin/yarn cache dir
/home/runner/.cache/yarn/v6
yarn cache is not found
7s
Run yarn install --frozen-lockfile
yarn install v1.22.21
[1/4] Resolving packages...
[2/4] Fetching packages...
error @release-it/conventional-changelog@[8](https://github.com/JamieMason/eslint-plugin-prefer-arrow-functions/actions/runs/8047862700/job/21977992063?pr=31#step:3:9).0.[1](https://github.com/JamieMason/eslint-plugin-prefer-arrow-functions/actions/runs/8047862700/job/21977992063?pr=31#step:4:1): The engine "node" is incompatible with this module. Expected version ">=18". Got "1[6](https://github.com/JamieMason/eslint-plugin-prefer-arrow-functions/actions/runs/8047862700/job/21977992063?pr=31#step:4:7).20.2"
error Found incompatible module.
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.
Error: Process completed with exit code 1.
Definitely seems unrelated to my change.
Also, the version in package.json is 3.3.0.
Thanks @mitchell-merry, are there any other test scenarios you can think of that would be worthwhile adding as well? Not specifically related to this bug, but to help us discover other potential ones.
/cc @renato-bohler
Not sure, sorry. I ran this in quite a large codebase and found no issues besides this when I ran. (I'm fairly new to custom eslint plugins, so not familiar with the edge cases)
Is there a reason why the node
parameter is node typed anywhere? Does ESLint not provide sufficient typing? :(
I suppose the node-version
in the github workflow needs to be updated to version 18 to get the build working. Feel free to push to this branch if you'd like.
I suppose the
node-version
in the github workflow needs to be updated to version 18 to get the build working. Feel free to push to this branch if you'd like.
if you merge main I've bumped that to 18
Not sure, sorry. I ran this in quite a large codebase and found no issues besides this when I ran. (I'm fairly new to custom eslint plugins, so not familiar with the edge cases)
By this I mean, can you think of other variations of valid syntax which could be relevant (async
, static
, named/not-named, does/not use this
etc) to add a test case for? Each test is just an example of what the code should be like before and after the plugin has been run.
Same question to @renato-bohler. The answer might be no by the way, I'm just raising it as thinking of these variations can find potential bugs really quickly that's all.
if you merge main I've bumped that to 18
done
By this I mean, can you think of other variations of valid syntax which could be relevant (async, static, named/not-named, does/not use this etc) to add a test case for? Each test is just an example of what the code should be like before and after the plugin has been run.
No, not at the moment.
If it's possible, could we get this PR in so as to fix the latest version of the package? Thank you
Description (What)
3.3.1 throws an error on this case:
because when it checks the
baz()
line, it gets the previous node, and assumes thatnode.declaration
is non-null. In this case (export { _foo as bar };
), it is. We can just return false in this case since it is not an overload.The error:
Justification (Why)
3.3.1 currently breaks when a module like above exists in a codebase.
How Can This Be Tested?
I added a unit test.
You can run eslint against a file with the contents above and confirm that it breaks before this change, and works after.