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(eslint-plugin): [no-invalid-this] crash when used with eslint 8.7.0 #4448

Merged
merged 4 commits into from Jan 17, 2022

Conversation

ota-meshi
Copy link
Contributor

PR Checklist

Overview

Changed not to call the deleted method to prevent the no-invalid-this rule from crashing. As far as I can see the implementation of the core rule, I think it works fine.

@typescript-eslint
Copy link
Contributor

Thanks for the PR, @ota-meshi!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!


🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day.

@nx-cloud
Copy link

nx-cloud bot commented Jan 17, 2022

☁️ Nx Cloud Report

CI ran the following commands for commit bb42059. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 48 targets

Sent with 💌 from NxCloud.

@netlify
Copy link

netlify bot commented Jan 17, 2022

❌ Deploy Preview for typescript-eslint failed.

🔨 Explore the source changes: bb42059

🔍 Inspect the deploy log: https://app.netlify.com/sites/typescript-eslint/deploys/61e527a619500d000796619a

@ota-meshi
Copy link
Contributor Author

The no-extra-semi rule has also changed and needs to be fixed. I will open PR.
eslint/eslint#15287

@bradzacher bradzacher added the bug Something isn't working label Jan 17, 2022
@bradzacher
Copy link
Member

@ota-meshi - feel free to fix it in this PR as well for ease of updating.

@bradzacher bradzacher added the awaiting response Issues waiting for a reply from the OP or another party label Jan 17, 2022
@ota-meshi
Copy link
Contributor Author

I reverted the version of eslint to make it easier to merge this PR.
We can later upgrade the eslint v8.7.0 with #4458.
We can see from the previous action result that no-invalid-this changed in this PR works with eslint v8.7.0.
https://github.com/typescript-eslint/typescript-eslint/runs/4837743893

@codecov
Copy link

codecov bot commented Jan 17, 2022

Codecov Report

Merging #4448 (bb42059) into main (d053cde) will decrease coverage by 0.05%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #4448      +/-   ##
==========================================
- Coverage   94.62%   94.57%   -0.06%     
==========================================
  Files         147      147              
  Lines        7849     7849              
  Branches     2511     2515       +4     
==========================================
- Hits         7427     7423       -4     
  Misses        233      233              
- Partials      189      193       +4     
Flag Coverage Δ
unittest 94.57% <0.00%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ackages/eslint-plugin/src/rules/no-invalid-this.ts 80.00% <0.00%> (-16.00%) ⬇️

@ota-meshi
Copy link
Contributor Author

I find it difficult for me to prevent a loss of coverage as the code covered changes with each version of eslint 😅

Copy link
Member

@bradzacher bradzacher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for this!

@bradzacher bradzacher removed the awaiting response Issues waiting for a reply from the OP or another party label Jan 17, 2022
@bradzacher bradzacher merged commit e56f1e5 into typescript-eslint:main Jan 17, 2022
@ota-meshi ota-meshi deleted the no-invalid-this branch January 17, 2022 08:58
lonyele pushed a commit to lonyele/typescript-eslint that referenced this pull request Feb 12, 2022
….0 (typescript-eslint#4448)

* fix(eslint-plugin): [no-invalid-this] crash when used with eslint 8.7.0

* revert eslint version
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

no-invalid-this issue exception after upgrading to eslint 8.7.0
2 participants