Set ESlint rules more strict 🚑 #911
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ndelangen
force-pushed
the
eslint-strict
branch
5 times, most recently
from
8e19de8 to
c241677
Compare
ndelangen
force-pushed
the
eslint-strict
branch
from
c241677 to
26f7ca5
Compare
| @@ -107,7 +107,7 @@ function previewProp(val) { | |||
| } | |||
|
|
|||
| if (!braceWrap) return content; | |||
| return <span>{{ content }}</span>; | |||
| return <span>{`{ ${content} }`}</span>; | |||
# Conflicts: # packages/addon-graphql/package.json # packages/addon-info/package.json # packages/addon-info/src/components/Node.js # packages/addon-knobs/package.json # packages/addon-notes/package.json # packages/channel-postmessage/package.json # packages/channel-websocket/package.json # packages/decorator-centered/package.json # packages/react-native-storybook/package.json # packages/react-storybook/src/server/config.js # packages/storybook-ui/package.json # packages/storybook-ui/src/modules/ui/components/layout/index.js
Codecov Report
@@ Coverage Diff @@
## master #911 +/- ##
==========================================
+ Coverage 13.31% 14.07% +0.76%
==========================================
Files 199 200 +1
Lines 4589 4497 -92
Branches 534 616 +82
==========================================
+ Hits 611 633 +22
+ Misses 3503 3333 -170
- Partials 475 531 +56
Continue to review full report at Codecov.
|
ndelangen
changed the title
|
@ndelangen I'm working on this and I just want to make sure this is right: I'm working directly off your branch -- not from my fork |
|
question: I've got this error: /Users/user/src/theinterned-storybook/packages/storybook-ui/src/modules/ui/components/left_panel/text_filter.test.js
1:1 error 'enzyme' should be listed in the project's dependencies. Run 'npm i -S enzyme' to add it import/no-extraneous-dependenciesthis is due to enzyme being in the root |
notes: 1. did not resolve `jsx-a11y` errors as that would require closer attention to the browser 2. did not resolve any `'enzyme' should be listed in the project's dependencies. Run 'npm i -S enzyme' to add it import/no-extraneous-dependencies` see note: #911 (comment)
adds `global` as the dep was missing in add-on-knobs
didn’t resolve jsx-a11y linting in this pass
uses `toMatchObject` rather than `toEqual` to be more lenient about what props are present
… the eslint config for globals
shilman
requested changes
Jun 6, 2017
.remarkrc
Outdated
| @@ -5,7 +5,8 @@ | |||
| ["remark-lint-code", {"js": { | |||
| "module": "node_modules/remark-lint-code-eslint", | |||
| "options": { | |||
| "fix": true | |||
| "fix": true, | |||
| "configFile": "/Users/dev/Projects/GitHub/storybook/react-storybook/.eslintrc-markdown.js" | |||
docs/pages/addons/api/index.md
Outdated
| @@ -26,7 +26,7 @@ See how we can use this: | |||
|
|
|||
| ```js | |||
| // Register the addon with a unique name. | |||
| addonAPI.register('kadira/notes', (storybookAPI) => { | |||
| addonAPI.register('my-organisation/my-addon', (storybookAPI) => { | |||
There was a problem hiding this comment.
@ndelangen can we put this in a different PR? i think it is unrelated to eslint?
+ make listing more specific
shilman
requested changes
Jun 6, 2017
There was a problem hiding this comment.
@ndelangen I am seeing an error trying to run gatsby in the docs
npm run dev
Huge string of errors:
ERROR in ./~/css-loader!./~/postcss-loader/lib!./components/Docs/Content/style.css
Module build failed: Error: No PostCSS Config found in: /Users/shilman/projects/storybook/new/storybook/docs/components/Docs/Content
at Error (native)
at /Users/shilman/projects/storybook/new/storybook/docs/node_modules/postcss-load-config/index.js:51:26
@ ./components/Docs/Content/style.css 4:14-130 18:2-22:4 19:20-136
...
# Conflicts: # app/react-native/src/manager/components/PreviewHelp.js
|
It's fixed @shilman ! |
|
I'm ready to merge this, who can test locally and approve? |
ndelangen
added
maintenance
This was referenced Nov 25, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue: eslint rules are too loose, errors are passing though.
What I did
As described in review comments #904 linting is rather non-strict right now.
Though I'm personally of the opinion too strict linting can be harmful, I agree, we could make it a bit more strict to weed out some bad code we have and prevent bad come from getting pushed to master.
.js&.jsxfiles.jsonfilessort-package-json(single time)..mdfileslint-stagedlint-stagedHow to test
run
npm run lint