Skip to content
GitLab
    • Explore Projects Groups Snippets
Projects Groups Snippets
  • /
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in / Register
  • C create-react-app
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
  • Issues 1,547
    • Issues 1,547
    • List
    • Boards
    • Service Desk
    • Milestones
  • Merge requests 417
    • Merge requests 417
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
  • Deployments
    • Deployments
    • Environments
    • Releases
  • Packages and registries
    • Packages and registries
    • Package Registry
    • Infrastructure Registry
  • Monitor
    • Monitor
    • Incidents
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Repository
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • Meta
  • create-react-app
  • Merge requests
  • !3087
An error occurred while fetching the assigned milestone of the selected merge_request.

chore: Allow to install eslint-plugin-react ^7.1.0 to get rid of warnings on start

  • Review changes

  • Download
  • Email patches
  • Plain diff
Administrator requested to merge github/fork/FezVrasta/master into master 7 years ago
  • Overview 13
  • Commits 1
  • Pipelines 0
  • Changes 1

Created by: FezVrasta

This should allow people to install eslint-plugin-react 7.2.0 which gets rid of the following warnings when you run yarn start:

can't resolve reference #/definitions/basicConfig from id #
can't resolve reference #/definitions/basicConfigOrBoolean from id #
can't resolve reference #/definitions/basicConfigOrBoolean from id #
Approval is optional

Merge details

  • 1 commit and 1 merge commit will be added to master.
  • Source branch will not be deleted.

Activity


  • Administrator
    Administrator @root · 7 years ago
    Author Contributor

    Created by: facebook-github-bot

    Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

    If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

  • Administrator
    Administrator @root · 7 years ago
    Author Contributor

    Created by: Timer

    What warnings are you talking about; could you please give me more context? These are development dependencies of a required package, so they should never be installed under normal use anyway.

  • Administrator added CLA Signed label 7 years ago

    added CLA Signed label

  • Administrator
    Administrator @root · 7 years ago
    Author Contributor

    Created by: facebook-github-bot

    Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

  • Administrator
    Administrator @root · 7 years ago
    Author Contributor

    Created by: FezVrasta

    So, when you start the app, you get the warnings reported in my first post.

    The problem is that yarn (and probably npm) will install 7.1.0 instead of any newer version because this is the only version that matches the requirements of all the packages.

    If you loosen the requirement here, the latest version will be installed and used by all the other packages of create-react-app as well.

    image

    ❯ cat node_modules/eslint-plugin-react/package.json | grep version
      "version": "7.1.0",

    If I put this in my package.json:

      "resolutions": {
        "eslint-plugin-react": "7.2.0"
      }

    The warnings go away

  • Administrator
    Administrator @root · 7 years ago
    Author Contributor

    Created by: Timer

    I guess more appropriately, I cannot reproduce this. A demo would be useful.

    Are you using Yarn, npm? What version? What react-scripts version (I assume latest; or is this the monorepo)?

    Once again, you're modifying a development dependency which should not affect the install tree (ever); did you mean to edit the package in react-scripts?

  • Administrator
    Administrator @root · 7 years ago
    Author Contributor

    Created by: FezVrasta

    Oh, yeah you are right, I should have edited the one in react-scripts.

    The warnings comes out after you eject and thinker a bit with the app, but the base problem is that react-scripts requires an older version without any particular reason.

  • Administrator
    Administrator @root · 7 years ago
    Author Contributor

    Created by: Timer

    react-scripts strictly depends on pinned versions of packages to prevent accidental breaking changes in package releases -- a common occurrence in the tooling world.

    I do not see us loosening this requirement as it's not clear to me how this benefits anything and how it couldn't be simply overridden when you eject (or how it causes this problem).

    If there's a problem here, it sounds like it's more in-line with invalid configuration or a breaking change released under a non-major version (and then stemming from configuration in eslint-config-react-app).

  • Administrator
    Administrator @root · 7 years ago
    Author Contributor

    Created by: FezVrasta

    Can't we bump it to 7.2.0 then? It simply fixes this warning, can't be anything bad.

  • Administrator
    Administrator @root · 7 years ago
    Author Contributor

    Created by: Timer

    I would like to figure out the underlying cause before we update packages.

  • Administrator
    Administrator @root · 7 years ago
    Author Contributor

    Created by: FezVrasta

    More info about the warning:

    • https://github.com/airbnb/javascript/issues/1488
    • https://github.com/yannickcr/eslint-plugin-react/issues/1290
  • Administrator
    Administrator @root · 7 years ago
    Author Contributor

    Created by: adam187

    @Timer @FezVrasta

    I had similar issue. In my case this warning did show when i was linting app with my own eslint config. my external config have eslint-plugin-react@7.4.0 as dependency but because react-scripts requires eslint-plugin-react@7.1.0 this was version that npm installed.

    adding

    "resolutions": {
        "eslint-plugin-react": "7.4.0"
      }

    to my package.json solved issue.

    But i didn't get this warring while starting server via yarn start

  • Administrator
    Administrator @root · 7 years ago
    Author Contributor

    Created by: react-scripts-dangerous

    Hello! I'm a bot that helps facilitate testing pull requests.

    Your pull request (commit d68fa770) has been released on npm for testing purposes.

    npm i react-scripts-dangerous@1.0.14-d68fa77.0
    # or
    yarn add react-scripts-dangerous@1.0.14-d68fa77.0
    # or
    create-react-app --scripts-version=react-scripts-dangerous@1.0.14-d68fa77.0 folder/

    Note that the package has not been reviewed or vetted by the maintainers. Only install it at your own risk!

    Thanks for your contribution!

  • Administrator
    Administrator @root · 7 years ago
    Author Contributor

    Created by: gaearon

    We will bump this plugin in the next major. It's not supported to install and use arbitrary versions of ESLint plugins in a non-ejected project.

  • Administrator closed 7 years ago

    closed

Please register or sign in to reply
0 Assignees
None
Assign to
0 Reviewers
None
Request review from
Labels
0
None
0
None
    Assign labels
  • Manage project labels

Milestone
No milestone
None
None
Time tracking
No estimate or time spent
Lock merge request
Unlocked
0
0 participants
Reference:
Source branch: github/fork/FezVrasta/master

Menu

Explore Projects Groups Snippets