-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Adds ESLint package @react-three/eslint-plugin #2698
Adds ESLint package @react-three/eslint-plugin #2698
Conversation
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit d67f238:
|
@@ -9,16 +9,8 @@ module.exports = { | |||
'<rootDir>/packages/test-renderer/dist', | |||
'<rootDir>/test-utils', | |||
], | |||
// coverageThreshold: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed this since it was commented out.
coverageDirectory: './coverage/', | ||
collectCoverage: true, | ||
collectCoverage: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Turned this to false since it was on for all unit tests, annoying! It's still enabled for yarn test
.
@@ -37,6 +37,7 @@ | |||
"validate": "preconstruct validate", | |||
"release": "yarn build && yarn changeset publish", | |||
"vers": "yarn changeset version", | |||
"codegen:eslint": "cd packages/eslint-plugin && yarn codegen", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does yarn@1 have workspaces-run inbuilt? Need to search over docs.
await writeFile(filepath, code) | ||
} | ||
|
||
async function generate() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Surprised this doesn't exist as a standalone npm pkg! Let me know if it does and my searchfu needs work.
@@ -0,0 +1,5 @@ | |||
--- | |||
'@react-three/eslint-plugin': minor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Figured we just keep bumping minors until we release a major version. This will release the package under v0.1.0.
@@ -0,0 +1,208 @@ | |||
import type { Rule } from 'eslint' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This module generates most content inside the eslint plugin, including: rule index, configs, and readme. It is a very simple implementation.
|
||
export default { | ||
plugins: ['@react-three'], | ||
rules: {}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No rules are recommended yet!
} | ||
|
||
await generateRuleIndex(rules) | ||
await generateConfig('all', rules) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we ever want to add another config it's just a matter of calling generate config again. Perhaps we could introduce a native config if that made sense?
@drcmda do we wait for @CodyJasonBennett to review before merging? |
No, all good here. Would be good to follow up with rules based on our docs for context/loop related patterns. |
@CodyJasonBennett no problem, can you link to the docs or even better list them here/in an issue? Will make it easier to not miss anything. For getting this merged can someone hit the merge button? Alternatively you can also give me access to merge 😉. |
I documented these in #2701. @drcmda can you give @itsdouges write & publishing access? |
Hey folks happy to make a start on some lint rules, would like to get this merged first to keep the changes scoped though. When you get a chance could we merge please? |
Thanks Cody 🙏 |
Hello! This pull request adds a new ESLint plugin package with no new rules (a stub one exists, the actual new rules will be added in a subsequent PR). Putting this up early for discussion, I think there is value to be had from writing lint rules for R3f, such as code that can be statically analyzed for problematic patterns (e.g clones in frame loops!).
For specific tasks done in the pull request I've:
@react-three/eslint-plugin
codegen
to generate the rules index, configs, plugin index, and README.yarn codegen:eslint
Tasks before merge
Subsequent tasks