Skip to content

Conversation

@ricardo-passthrough
Copy link

What:

Updating the pinned version of cosmiconfig

Why:

fix vulnerability report from npm audit:

# npm audit report

yaml  <2.2.2
Severity: moderate
Uncaught Exception in yaml - https://github.com/advisories/GHSA-f9xv-q969-pqx4
fix available via `npm audit fix --force`
Will install @emotion/[email protected], which is a breaking change
node_modules/yaml
  cosmiconfig  6.0.0 - 7.1.0
  Depends on vulnerable versions of yaml
  node_modules/cosmiconfig
    babel-plugin-macros  >=2.6.2
    Depends on vulnerable versions of cosmiconfig
    node_modules/babel-plugin-macros
      @emotion/babel-plugin  >=11.9.5
      Depends on vulnerable versions of babel-plugin-macros
      node_modules/@emotion/babel-plugin
        @emotion/react  >=11.10.0
        Depends on vulnerable versions of @emotion/babel-plugin
        node_modules/@emotion/react
        @emotion/styled  >=11.10.0
        Depends on vulnerable versions of @emotion/babel-plugin
        node_modules/@emotion/styled

6 moderate severity vulnerabilities

How:

  1. update the packages on package.json
  2. adjusted some tests for minor cosmetic changes on snapshots

Checklist:

  • Documentation NA
  • Tests NA
  • Ready to be merged

"license": "MIT",
"dependencies": {
"cosmiconfig": "^7.0.0",
"cosmiconfig": "^8.1.3",
Copy link
Collaborator

Choose a reason for hiding this comment

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

cosmiconfig 8 drops support for node 10. I'm looking at engines lower in this file and it seems like babel-plugin-macros still supports node 10, which would mean that it would have to drop that support in order to adopt cosmiconfig 8. I believe that dropping support for older node versions is something that will need to happen in a new major version, as it did for cosmiconfig.

@conartist6
Copy link
Collaborator

Also thanks for contributing this! A new major version isn't necessarily a blocker if this is the right technical choice, but I want to make sure that if the library is doing a major version bump that all the choices have been considered first. I think that also means reviewing the trade-offs between this and #183.

@j-pollack
Copy link

What about making this a patch level change, while y'all consider dropping yaml support entirely (#183) a major/breaking change?

@conartist6
Copy link
Collaborator

@jonathan3692bf I am inclined to do nothing for now, since it was my determination over on #192 that this PR is an attempt to fix an issue that does not exist.

@j-pollack
Copy link

You bring up a good point in #192; updating this would be the equivalent of "sanitation theater"...

@ricardo-passthrough
Copy link
Author

closing the PR for now, thanks for the discussion!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants