ESLint support for documentation #3047

abdulhannanali · 2017-03-02T20:16:02Z

Summary

Add Eslint support for documentation using the plugin eslint-plugin-markdown to lint the code in js code blocks. The PR intends to solve #3028 , by linting preexisting example code in Jest docs folder according to the JavaScript standards Jest uses, and also adds a script to automatically lint code in future using build step. The Markdown files in the docs/ folder can be linted using the script yarn run lint-docs

Test plan

No changes to the code, The code makes some changes to the docs, the following commands pass on this code

cd website
yarn run test

and for the Jest code

yarn run test
yarn run test-ci
yarn run test-ci-partial

maximderbin · 2017-03-02T20:26:30Z

examples/enzyme/yarn-error.log

@@ -0,0 +1,43 @@
+Arguments: 


Remove this file and update .gitignore ?

Ooops How did this get in...

Thanks fixed!

abdulhannanali · 2017-03-02T20:49:39Z

@DmitriiAbramov The lint-docs script which lints the documentation using eslint-plugin-markdown is added to scripts yarn run test and yarn run test-ci, so I don't need to change the circle.yml file in any way, and they run in an order, where first the checks are performed on the code, and if everything works fine, documentation is checked for the errors.

Should I run this separately in circle.yml file, or is it fine including it with test and test-ci scripts. You can check package.json for more info.

abdulhannanali · 2017-03-02T21:20:44Z

According to Dmitrii! it can be included in test as running test script means usually, that everything is working fine, and there are no major errors that are breaking the tests or code. It includes docs as well. Removing this from test-ci as this seems unnecessary. 👍

abdulhannanali · 2017-03-02T21:44:56Z

docs/GlobalAPI.md

@@ -195,6 +195,8 @@ Also under the alias: `fdescribe(name, fn)`
 You can use `describe.only` if you want to run only one describe block:

 ```js
+
+/* eslint jest/no-focused-tests: "off" */


As this is the description of describe.only itself, we have to explicitly turn off the jest/no-focused-tests rule for this one by adding a comment. If this causes confusion in the documentation, can we turn off this rule globally?

yeah. i think we should disable it for the whole .md run

abdulhannanali · 2017-03-02T21:45:32Z

docs/GlobalAPI.md

@@ -281,13 +283,15 @@ When you are debugging a large codebase, you will often only want to run a subse
 For example, let's say you had these tests:

 ```js
+/* eslint jest/no-focused-tests: "off" */


Another explicit eslint comment

abdulhannanali · 2017-03-02T21:46:29Z

docs/ManualMocks.md

-  views
-  config
+```bash
+.


Generated this tree using tree command. Want to keep it?

that looks good!

abdulhannanali · 2017-03-02T21:48:06Z

docs/MockFunctions.md

 bound();

 console.log(myMock.mock.instances);
-> [ <a>, <b> ]
+// > [ <a>, <b> ]


@DmitriiAbramov The syntax of // > itself is quite different from the other files, and is only used in this one MockFunctions.md file. Adding comment is necessary otherwise eslint breaks.

aaronabramov · 2017-03-03T02:13:27Z

docs/GlobalAPI.md

@@ -195,6 +195,8 @@ Also under the alias: `fdescribe(name, fn)`
 You can use `describe.only` if you want to run only one describe block:

 ```js
+
+/* eslint jest/no-focused-tests: "off" */


yeah. i think we should disable it for the whole .md run

aaronabramov · 2017-03-03T02:15:03Z

docs/ManualMocks.md

-  views
-  config
+```bash
+.


that looks good!

aaronabramov · 2017-03-03T02:18:44Z

package.json

    "postinstall": "node ./scripts/postinstall.js && node ./scripts/build.js && (cd packages/eslint-plugin-jest && yarn link) && yarn link eslint-plugin-jest",
    "publish": "yarn run build-clean && yarn run build && lerna publish",
    "test-ci": "yarn run typecheck && yarn run lint && yarn run build && yarn run jest-coverage -- -i && yarn run test-examples && node scripts/mapCoverage.js && codecov",
    "test-ci-partial": "yarn run build && yarn run jest -- -i && yarn run test-examples",
    "test-examples": "node scripts/test_examples.js",
    "test-pretty-format-perf": "node packages/pretty-format/perf/test.js",
-    "test": "yarn run typecheck && yarn run lint && yarn run build && yarn run jest && yarn run test-examples",
+    "test": "yarn run typecheck && yarn run lint && yarn run build && yarn run jest && yarn run test-examples && yarn run lint-docs",


we should probably put it after yarn run lint. You don't want to wait for the entire test suite to run to find out that you have 1 lint error in the docs :)

Cool, Thanks will do, makes sense

aaronabramov · 2017-03-03T02:20:13Z

docs/MockFunctions.md

 bound();

 console.log(myMock.mock.instances);
-> [ <a>, <b> ]
+// > [ <a>, <b> ]


aaronabramov

it looks awesome!! it'll help us keep the docs clean!
just a few changes and i think it's good to go!

abdulhannanali · 2017-03-03T13:49:44Z

@DmitriiAbramov Thank you for pointing out the changes, I have move the jest and react specific configurations to the top, so we don't have to specify them within the code blocks, which can get confusing for the readers. The --rules in package.json was getting bloated, so I have moved the rules to an additional .eslintrc-docs.json file to override the rules present in .eslintrc.

abdulhannanali · 2017-03-03T13:50:37Z

I changed the order for running lint-docs after lint and and also added it to test-ci so it's tested by the continuous integration system too. :)

abdulhannanali · 2017-03-03T13:50:45Z

Sorry closed this by mistake

cpojer · 2017-03-03T14:02:34Z

This is awesome @abdulhannanali! I'll let @DmitriiAbramov merge this one :) Would you mind resolving the conflict in the yarn lockfile?

test script already includes lint-docs, it'll be reduntant to do the same for more than one node versions

Removing eslint comments from docs and moving them to top level in .eslintrc-docs.json

cpojer · 2017-03-04T08:46:45Z

Awesome work. Thank you!

cpojer · 2017-03-04T08:47:23Z

@abdulhannanali what do you think of also enabling this set of lint rules to the "examples/" folder? The code there should be the same as in the documentation :)

abdulhannanali · 2017-03-04T14:35:04Z

@cpojer Sorry for not resolving the merge conflict at time. Yes it can definitely be done. Thanks a lot for merging 😄

abdulhannanali · 2017-03-06T06:23:21Z

@cpojer Most of the rules we have added in eslint-docs.json are there because of the code examples in the docs can't run standalone except the consistent-return as this was just annoying. However examples are standalone examples, we can run. I see no point of adopting the same linting rules for examples/.

abdulhannanali · 2017-03-06T06:24:50Z

@cpojer These are the rules we are using

{
    "rules": {
        "react/react-in-jsx-scope": 0,
        "react/jsx-no-undef": 0,
        "jest/no-focused-tests": 0,
        "no-undef": 0,
        "no-unused-vars": 0,
        "consistent-return": 0
    },
    "description": ".eslintrc-docs.json overrides the rules specified in .eslintrc, to make it more suitable for running on code examples in docs/ folder"
}

cpojer · 2017-03-06T09:12:03Z

@abdulhannanali I mostly recommended that to make sure that the style in the examples matches those in the documentation.

abdulhannanali · 2017-03-06T09:22:44Z

@cpojer As far as the apparent styles are concerned, they are definitely going to match. Since both use the Fb Linting Standards. The rules for the docs are more concerned with validation errors we get in the docs. :)

cpojer · 2017-03-06T09:27:37Z

Oh, you are right. We are linting everything but the react-native example for some reason :) I thought we ignored them all.

abdulhannanali · 2017-03-06T18:25:47Z

@cpojer #3080 fixes it 😄

* add markdown plugin in eslint * add lint-docs build step * lint JavaScript file * final lint-docs command * lint ExpectAPI * lint GettingStarted.md * lint GlobalAPI * lint GlobalAPI * lint JestObjectAPI * lint Webpack.md * lint UsingMatchers.md in docs * lint TutorialReactNative.md in docs * lint TutorialReact.md in docs * lint docs file * add lint-docs to test-ci script * add lint-docs script to test & test-ci * turn off more rules to lint-docs in package.json * remove lint-docs from test and test-ci * add lint-docs to test script in package.json * remove lint-docs script from circle.yml * add lint-docs script to test-ci in package.json * rename js codeblock with json appropriately * remove unnecessary log files * remove lint-docs from test-ci test script already includes lint-docs, it'll be reduntant to do the same for more than one node versions * remove error.log file * use eslintrc-docs.json for overriding * remove eslint comments from docs Removing eslint comments from docs and moving them to top level in .eslintrc-docs.json * add lint-docs in correct order to test-ci and test

github-actions · 2021-05-13T21:06:53Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

facebook-github-bot added the CLA Signed ✔️ label Mar 2, 2017

maximderbin reviewed Mar 2, 2017

View reviewed changes

abdulhannanali commented Mar 2, 2017

View reviewed changes

aaronabramov reviewed Mar 3, 2017

View reviewed changes

aaronabramov suggested changes Mar 3, 2017

View reviewed changes

abdulhannanali closed this Mar 3, 2017

abdulhannanali reopened this Mar 3, 2017

aaronabramov approved these changes Mar 4, 2017

View reviewed changes

abdulhannanali added 13 commits March 4, 2017 08:45

add markdown plugin in eslint

ba946c4

add lint-docs build step

b56ca27

lint JavaScript file

332a17f

final lint-docs command

a31cfcb

lint ExpectAPI

ac36553

lint GettingStarted.md

bc078f3

lint GlobalAPI

7c60fdb

lint GlobalAPI

c930511

lint JestObjectAPI

60ad96a

lint Webpack.md

bb4b33e

lint UsingMatchers.md in docs

9f9ec1f

lint TutorialReactNative.md in docs

c4ee489

lint TutorialReact.md in docs

d3181c0

abdulhannanali added 14 commits March 4, 2017 08:45

add lint-docs to test-ci script

34ec518

add lint-docs script to test & test-ci

7184d5c

turn off more rules to lint-docs in package.json

09b837a

remove lint-docs from test and test-ci

a67a26f

add lint-docs to test script in package.json

3e60686

remove lint-docs script from circle.yml

c0ca118

add lint-docs script to test-ci in package.json

928e122

rename js codeblock with json appropriately

ed693f8

remove unnecessary log files

51bf062

remove lint-docs from test-ci

f6f8aaf

test script already includes lint-docs, it'll be reduntant to do the same for more than one node versions

remove error.log file

2af605a

use eslintrc-docs.json for overriding

8d83aff

remove eslint comments from docs

4b2a072

Removing eslint comments from docs and moving them to top level in .eslintrc-docs.json

add lint-docs in correct order to test-ci and test

42406cc

cpojer force-pushed the lint-docs-code branch from 18ede03 to 42406cc Compare March 4, 2017 08:46

cpojer merged commit 5ab8e28 into jestjs:master Mar 4, 2017

abdulhannanali mentioned this pull request Mar 6, 2017

lint all examples using eslint #3080

Merged

github-actions bot locked as resolved and limited conversation to collaborators May 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ESLint support for documentation #3047

ESLint support for documentation #3047

abdulhannanali commented Mar 2, 2017

maximderbin Mar 2, 2017

abdulhannanali Mar 2, 2017

abdulhannanali Mar 2, 2017

abdulhannanali commented Mar 2, 2017

abdulhannanali commented Mar 2, 2017

abdulhannanali Mar 2, 2017 •

edited

Loading

aaronabramov Mar 3, 2017

abdulhannanali Mar 2, 2017

abdulhannanali Mar 2, 2017

aaronabramov Mar 3, 2017

abdulhannanali Mar 2, 2017

aaronabramov Mar 3, 2017

aaronabramov Mar 3, 2017

aaronabramov Mar 3, 2017

aaronabramov Mar 3, 2017

abdulhannanali Mar 3, 2017

aaronabramov Mar 3, 2017

aaronabramov left a comment

abdulhannanali commented Mar 3, 2017 •

edited

Loading

abdulhannanali commented Mar 3, 2017

abdulhannanali commented Mar 3, 2017

cpojer commented Mar 3, 2017

cpojer commented Mar 4, 2017

cpojer commented Mar 4, 2017

abdulhannanali commented Mar 4, 2017

abdulhannanali commented Mar 6, 2017

abdulhannanali commented Mar 6, 2017

cpojer commented Mar 6, 2017

abdulhannanali commented Mar 6, 2017

cpojer commented Mar 6, 2017

abdulhannanali commented Mar 6, 2017

github-actions bot commented May 13, 2021

ESLint support for documentation #3047

ESLint support for documentation #3047

Conversation

abdulhannanali commented Mar 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abdulhannanali commented Mar 2, 2017

abdulhannanali commented Mar 2, 2017

abdulhannanali Mar 2, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aaronabramov left a comment

Choose a reason for hiding this comment

abdulhannanali commented Mar 3, 2017 • edited Loading

abdulhannanali commented Mar 3, 2017

abdulhannanali commented Mar 3, 2017

cpojer commented Mar 3, 2017

cpojer commented Mar 4, 2017

cpojer commented Mar 4, 2017

abdulhannanali commented Mar 4, 2017

abdulhannanali commented Mar 6, 2017

abdulhannanali commented Mar 6, 2017

cpojer commented Mar 6, 2017

abdulhannanali commented Mar 6, 2017

cpojer commented Mar 6, 2017

abdulhannanali commented Mar 6, 2017

github-actions bot commented May 13, 2021

abdulhannanali Mar 2, 2017 •

edited

Loading

abdulhannanali commented Mar 3, 2017 •

edited

Loading