Set ESlint rules more strict 🚑 by ndelangen · Pull Request #911 · storybookjs/storybook

ndelangen · 2017-04-17T14:44:39Z

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.

How to test

run npm run lint

darekrossman · 2017-04-28T13:33:54Z


  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 · 2017-04-28T15:26:32Z

Codecov Report

Merging #911 into master will increase coverage by 0.76%.
The diff coverage is 15.24%.

@@            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

Impacted Files	Coverage Δ
addons/knobs/src/components/Panel.js	`88% <ø> (ø)`	⬆️
lib/ui/src/modules/ui/containers/shortcuts_help.js	`100% <ø> (ø)`	⬆️
...ct-native/src/server/config/webpack.config.prod.js	`0% <ø> (ø)`	⬆️
addons/options/preview.js	`0% <ø> (ø)`	⬆️
...p/react-native/src/server/config/webpack.config.js	`0% <ø> (ø)`	⬆️
addons/options/src/manager/index.js	`0% <ø> (ø)`	⬆️
lib/ui/src/modules/ui/configs/handle_keyevents.js	`33.33% <ø> (ø)`	⬆️
addons/info/src/index.js	`0% <ø> (ø)`	⬆️
addons/comments/manager.js	`0% <ø> (ø)`	⬆️
addons/options/manager.js	`0% <ø> (ø)`	⬆️
... and 143 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a5de77a...432c371. Read the comment docs.

theinterned · 2017-04-28T18:20:14Z

@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

theinterned · 2017-04-28T18:48:19Z

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-dependencies

this is due to enzyme being in the root package.json is it appropriate just add an eslint-disable-line or should we modify the eslintrc?

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

FIX linting

shilman

Hey this looks great but I think we should get it merged and then do refinements on the merged version. I think this PR is getting out of control?

shilman · 2017-06-06T08:50:38Z

      "options": {
-        "fix": true
+        "fix": true,
+        "configFile": "/Users/dev/Projects/GitHub/storybook/react-storybook/.eslintrc-markdown.js"


@ndelangen ???

Will fix this

shilman · 2017-06-06T08:51:49Z

 ```js
 // Register the addon with a unique name.
-addonAPI.register('kadira/notes', (storybookAPI) => {
+addonAPI.register('my-organisation/my-addon', (storybookAPI) => {


@ndelangen can we put this in a different PR? i think it is unrelated to eslint?

+ make listing more specific

shilman

@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

ndelangen · 2017-06-06T18:57:47Z

It's fixed @shilman !

ndelangen · 2017-06-06T18:58:28Z

I'm ready to merge this, who can test locally and approve?

ADD stricter eslint rules

0a7163b

ndelangen added the in progress label Apr 17, 2017

ndelangen changed the title ~~Eslint strict~~ Set ESlint rules more strict Apr 17, 2017

ndelangen added the ci: do not merge label Apr 17, 2017

ndelangen force-pushed the eslint-strict branch 5 times, most recently from 8e19de8 to c241677 Compare April 18, 2017 20:51

FIX linting

26f7ca5

ndelangen force-pushed the eslint-strict branch from c241677 to 26f7ca5 Compare April 23, 2017 20:51

FIX for fc7e09b?diff=split#commitcomment-21940155

a37eb11

darekrossman reviewed Apr 28, 2017

View reviewed changes

ndelangen added 2 commits April 28, 2017 15:39

FIX linting

b801d6f

ndelangen changed the title ~~Set ESlint rules more strict~~ Set ESlint rules more strict 🚑 Apr 28, 2017

fixes linting in storyshots

d22d23a

theinterned added 10 commits April 28, 2017 17:41

fix: failing Panel.js test in add-knobs

84f1e37

adds `global` as the dep was missing in add-on-knobs

resolves listing in Stories component of storybook-ui

5deb96e

didn’t resolve jsx-a11y linting in this pass

fix: test for storybook-ui let_panel/index

59daa5c

uses `toMatchObject` rather than `toEqual` to be more lenient about what props are present

fix: eslint -- resolves globals using the global module rather than…

0d2cab0

… the eslint config for globals

fix: linting in react-storybook package

5219498

fix: adds missing ws dep to react-native-storybook

0f65908

fix: adds missing global dep to react-native-storybook

55a67e7

fix: linting in react-native-storybook package

52b8490

fix: listing in getstorybook package

90edfdf

ndelangen added 6 commits June 5, 2017 11:33

Merge branch 'master' into eslint-strict

e40ef62

FIX after merge

baecbf7

ADD eslint config for examples inside markdown

6c83d97

FIX linting

FIX changelog markdown

04a0551

FIX linting && ADD globals to makrdown code examples lint config

3d87b27

FIX linting

dfabb18

shilman requested changes Jun 6, 2017

View reviewed changes

Add missing import

0ecb1bf

+ make listing more specific

shilman requested changes Jun 6, 2017

View reviewed changes

ndelangen added 6 commits June 6, 2017 13:19

ADD linting on some rc files, CHANGe to js rc for remark

7db3cc0

FIX linting

d75bcb8

FIX linting

e4e639e

ADD eslint config for .json files && FIX linting

921daab

Merge branch 'master' into eslint-strict

7765c45

# Conflicts: # app/react-native/src/manager/components/PreviewHelp.js

CHANGE all package.jsons so they are uniformly sorted

28a4b32

ndelangen force-pushed the eslint-strict branch from 3211025 to 28a4b32 Compare June 6, 2017 15:47

ndelangen added 2 commits June 6, 2017 18:03

FIX docs

d1aa2ed

CHANGE path to markdown code example eslint config to relative path

ae5dae9

ndelangen added this to the v3.0.2 milestone Jun 6, 2017

ndelangen self-assigned this Jun 6, 2017

ndelangen added maintenance User-facing maintenance tasks and removed ci: do not merge in progress labels Jun 6, 2017

Merge branch 'master' into eslint-strict

432c371

ndelangen merged commit 67d6af3 into master Jun 6, 2017

ndelangen deleted the eslint-strict branch June 6, 2017 20:20

This was referenced Nov 25, 2023

[Snyk-dev] Fix for 24 vulnerabilities lili2311/storybook#200

Open

[Snyk-dev] Fix for 1 vulnerabilities lili2311/storybook#244

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Set ESlint rules more strict 🚑#911

Set ESlint rules more strict 🚑#911
ndelangen merged 71 commits intomasterfrom
eslint-strict

ndelangen commented Apr 17, 2017 •

edited

Loading

Uh oh!

darekrossman Apr 28, 2017

Uh oh!

codecov bot commented Apr 28, 2017 •

edited

Loading

Uh oh!

theinterned commented Apr 28, 2017

Uh oh!

theinterned commented Apr 28, 2017 •

edited

Loading

Uh oh!

shilman left a comment

Uh oh!

shilman Jun 6, 2017

Uh oh!

ndelangen Jun 6, 2017

Uh oh!

shilman Jun 6, 2017

Uh oh!

shilman left a comment

Uh oh!

ndelangen commented Jun 6, 2017

Uh oh!

ndelangen commented Jun 6, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

ndelangen commented Apr 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What I did

How to test

Uh oh!

darekrossman Apr 28, 2017

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Apr 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

theinterned commented Apr 28, 2017

Uh oh!

theinterned commented Apr 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shilman left a comment

Choose a reason for hiding this comment

Uh oh!

shilman Jun 6, 2017

Choose a reason for hiding this comment

Uh oh!

ndelangen Jun 6, 2017

Choose a reason for hiding this comment

Uh oh!

shilman Jun 6, 2017

Choose a reason for hiding this comment

Uh oh!

shilman left a comment

Choose a reason for hiding this comment

Uh oh!

ndelangen commented Jun 6, 2017

Uh oh!

ndelangen commented Jun 6, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ndelangen commented Apr 17, 2017 •

edited

Loading

codecov bot commented Apr 28, 2017 •

edited

Loading

theinterned commented Apr 28, 2017 •

edited

Loading