Skip to content

fix: add prefer-in-document to recommended config #112

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

Merged
merged 2 commits into from
Dec 2, 2020

Conversation

nickserv
Copy link
Member

@nickserv nickserv commented Nov 30, 2020

What: add prefer-in-document to recommended config

Why: this is a useful rule that's in line with other best practices that the recommended config follows

How: enabled recommended flag

Checklist:

  • Documentation
  • Tests N/A
  • Ready to be merged

@codecov
Copy link

codecov bot commented Nov 30, 2020

Codecov Report

Merging #112 (e3eb694) into master (c397bd1) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #112   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           12        12           
  Lines          247       247           
  Branches        37        37           
=========================================
  Hits           247       247           
Impacted Files Coverage Δ
src/rules/prefer-in-document.js 100.00% <ø> (ø)

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 c397bd1...e3eb694. Read the comment docs.

@nickserv nickserv force-pushed the prefer-in-document-recommended branch from 7a27e87 to 6f24739 Compare November 30, 2020 13:02
Copy link
Contributor

@AntonNiklasson AntonNiklasson left a comment

Choose a reason for hiding this comment

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

I agree it should be recommended. I'll leave it up to Ben to decide though.

@nickserv nickserv requested a review from benmonro November 30, 2020 13:25
@AriPerkkio
Copy link
Contributor

AriPerkkio commented Nov 30, 2020

@nickmccurdy before adding this rule as recommended check the latest added smoke tests, PR. prefer-in-document is crashing on many repositories. Here's one example:

Rule: prefer-in-document

  • Message: Cannot read property 'name' of undefined Occurred while linting <text>:159
  • Path: gatsbyjs/gatsby/integration-tests/structured-logging/__tests__/logs.js
  • Link
        event.type === `LOG_ACTION` &&
        event.action.type === `ACTIVITY_END` &&
        event.action.payload.id === `createPages`
    )
    expect(createPagesEndActivityEvent).toBeDefined()
    expect(createPagesEndActivityEvent.action.payload.status).toBe(`FAILED`)
  })
  ;[`success`, `info`, `warn`, `log`, `error`].forEach(level => {
    // success, info and log all emit `INFO`
    const mapActionToLevel = {
TypeError: Cannot read property 'name' of undefined
Occurred while linting <text>:159
    at check (/<root-path-removed>/eslint-plugin-jest-dom/dist/rules/prefer-in-document.js:39:27)
    at CallExpression[callee.object.callee.name='expect'][callee.property.name=/(toHaveLength|toBeDefined|toBeNull)/] (/<root-path-removed>/eslint-plugin-jest-dom/dist/rules/prefer-in-document.js:97:7)
    at /<root-path-removed>/eslint-plugin-jest-dom/node_modules/eslint/lib/linter/safe-emitter.js:45:58
    at Array.forEach (<anonymous>)
    at Object.emit (/<root-path-removed>/eslint-plugin-jest-dom/node_modules/eslint/lib/linter/safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector (/<root-path-removed>/eslint-plugin-jest-dom/node_modules/eslint/lib/linter/node-event-generator.js:254:26)
    at NodeEventGenerator.applySelectors (/<root-path-removed>/eslint-plugin-jest-dom/node_modules/eslint/lib/linter/node-event-generator.js:283:22)
    at NodeEventGenerator.enterNode (/<root-path-removed>/eslint-plugin-jest-dom/node_modules/eslint/lib/linter/node-event-generator.js:297:14)
    at CodePathAnalyzer.enterNode (/<root-path-removed>/eslint-plugin-jest-dom/node_modules/eslint/lib/linter/code-path-analysis/code-path-analyzer.js:711:23)
    at /<root-path-removed>/eslint-plugin-jest-dom/node_modules/eslint/lib/linter/linter.js:952:32

@benmonro
Copy link
Member

let's hold off on merging this until #107 is implemented and we've had a chance to smoke test it more to make sure there's nothing crazy out there.

@benmonro
Copy link
Member

benmonro commented Dec 2, 2020

all the smoke test issues have been resolved. changing to recommended. 👍

@benmonro benmonro merged commit dfe85c0 into master Dec 2, 2020
@benmonro benmonro deleted the prefer-in-document-recommended branch December 2, 2020 04:53
@github-actions
Copy link

github-actions bot commented Dec 2, 2020

🎉 This PR is included in version 3.4.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants