[Hold for payment 2022-11-15] [$250] Update @svgr/webpack to version 6.0.0

[flodnv]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Problem

The package nth-check@1.0.2 has a security vulnerability introduced through @svgr/webpack@5.5.0, fixed in nth-check@2.0.1

$ npm list nth-check                                                                                                                                                                                                                                    [12:38:18]
new.expensify@1.2.14-0 /Users/flo/Expensidev/App
├─┬ @storybook/react@6.5.10
│ └─┬ @storybook/core@6.5.10
│   └─┬ @storybook/core-server@6.5.10
│     ├─┬ @storybook/builder-webpack4@6.5.10
│     │ └─┬ html-webpack-plugin@4.5.2
│     │   └─┬ pretty-error@2.1.2
│     │     └─┬ renderkid@2.0.7
│     │       └─┬ css-select@4.3.0
│     │         └── nth-check@2.1.1 deduped
│     └─┬ @storybook/manager-webpack4@6.5.10
│       └─┬ html-webpack-plugin@4.5.2
│         └─┬ pretty-error@2.1.2
│           └─┬ renderkid@2.0.7
│             └─┬ css-select@4.3.0
│               └── nth-check@2.1.1 deduped
├─┬ @svgr/webpack@5.5.0
│ └─┬ @svgr/plugin-svgo@5.5.0
│   └─┬ svgo@1.3.2
│     └─┬ css-select@2.1.0
│       └── nth-check@1.0.2
├─┬ html-webpack-plugin@5.5.0
│ └─┬ pretty-error@4.0.0
│   └─┬ renderkid@3.0.0
│     └─┬ css-select@4.3.0
│       └── nth-check@2.1.1 deduped
└─┬ react-native-svg@12.4.4
  └─┬ css-select@5.1.0
    └── nth-check@2.1.1

Solution

Upgrade to @svgr/webpack@6.0.0

Expensify/Expensify Issue URL: https://github.com/Expensify/Expensify/issues/218325 Upwork URL: https://www.upwork.com/jobs/~01615065cda2c02b5a

View all open jobs on GitHub

[melvin-bot[bot]]

Triggered auto assignment to @flaviadefaria (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[melvin-bot[bot]]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @Santhosh-Sellavel (External)

[melvin-bot[bot]]

Triggered auto assignment to @Gonals (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[hungvu193]

Proposal

Solution: Upgrade version at package.json.

-  "@svgr/webpack": "^5.5.0",
+  "@svgr/webpack"  "^6.0.0",

[trjExpensify]

I have a related one, going to take this from you @flaviadefaria.

[trjExpensify]

Upwork job here: https://www.upwork.com/jobs/~01615065cda2c02b5a

[gadhiyamanan]

Proposal

update from "@svgr/webpack": "^5.5.0" to "@svgr/webpack": "^6.0.0" in package.json https://github.com/Expensify/App/blob/8aec877a678d09600e110e0fc7bc597e342ef342/package.json#L138

BREAKING CHANGES

1. Config format of SVGO changes & SVGR does not merge SVGO config
2. Template has a new format
3. core: @svgr/core now exposes { transform } instead of default export
4. using --icon as latest arg now requires "--"

[saivineeth100]

When I raised the Same issue long back , it was not considered I even shared the issue in slack, I included other package also ( react-native-svg-transformer) it also uses svgo which is dependent on older version of nth check

[flodnv]

FWIW, the problem statement is not the same.

[saivineeth100]

Heading is different , but its same When I posted about @svgr/webpack issue I was said that's not useful p/s Check comments here https://expensify.slack.com/archives/C01GTK53T8Q/p1660141537100609?thread_ts=1660141537.100609&cid=C01GTK53T8Q

[flodnv]

As far as I can tell, that problem statement and the one proposed here are not the same. Am I wrong?

[saivineeth100]

Ok , what about This This has similar security vulnerability

[saivineeth100]

Ok , what about This

This has similar security vulnerability

[melvin-bot[bot]]

@trjExpensify, @Gonals, @Santhosh-Sellavel Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[flodnv]

@saivineeth100 according to package-lock.json andnpm list nth-check`, this is not accurate:

$ npm list nth-check                                                                                                                                                                                                                                 
new.expensify@1.2.16-4 /Users/flo/Expensidev/App
├─┬ @storybook/react@6.5.10
│ └─┬ @storybook/core@6.5.10
│   └─┬ @storybook/core-server@6.5.10
│     ├─┬ @storybook/builder-webpack4@6.5.10
│     │ └─┬ html-webpack-plugin@4.5.2
│     │   └─┬ pretty-error@2.1.2
│     │     └─┬ renderkid@2.0.7
│     │       └─┬ css-select@4.3.0
│     │         └── nth-check@2.1.1 deduped
│     └─┬ @storybook/manager-webpack4@6.5.10
│       └─┬ html-webpack-plugin@4.5.2
│         └─┬ pretty-error@2.1.2
│           └─┬ renderkid@2.0.7
│             └─┬ css-select@4.3.0
│               └── nth-check@2.1.1 deduped
├─┬ @svgr/webpack@5.5.0
│ └─┬ @svgr/plugin-svgo@5.5.0
│   └─┬ svgo@1.3.2
│     └─┬ css-select@2.1.0
│       └── nth-check@1.0.2
├─┬ html-webpack-plugin@5.5.0
│ └─┬ pretty-error@4.0.0
│   └─┬ renderkid@3.0.0
│     └─┬ css-select@4.3.0
│       └── nth-check@2.1.1 deduped
└─┬ react-native-svg@12.4.4
  └─┬ css-select@5.1.0
    └── nth-check@2.1.1

I would appreciate it if we could please keep this issue's scope to the problem/solution description in the issue.

[flodnv]

@Santhosh-Sellavel can you please review the proposals from @hungvu193 and @gadhiyamanan

[Santhosh-Sellavel]

@hungvu193 @gadhiyamanan

Do we have any other migration changes expected as there are some breaking changes?

Can you elaborate, on changes, if there are some migration changes, and how it does handle?

if no changes are required, how/why the breaking changes don't affect our app?

Thanks!

[hungvu193]

@Santhosh-Sellavel Since we are using @svgr/webpack as a loader for web and all these breaking changes for the command line tool, template, @svgr/core which we didn't use in our application, we only need to update the version in package.json.

[Santhosh-Sellavel]

@flodnv @Gonals

@hungvu193 proposal looks good to me.

C+ Reviewed. 👀 🎀 👀

Seems we need to update another library also, react-native-svg-transformer library's dependency libraries css select uses nth-check@1.0.2

new.expensify@1.2.14-0 /Users/santhoshkumar/Documents/App
├─┬ @storybook/react@6.5.10
│ └─┬ @storybook/core@6.5.10
│   └─┬ @storybook/core-server@6.5.10
│     ├─┬ @storybook/builder-webpack4@6.5.10
│     │ └─┬ html-webpack-plugin@4.5.2
│     │   └─┬ pretty-error@2.1.2
│     │     └─┬ renderkid@2.0.7
│     │       └─┬ css-select@4.3.0
│     │         └── nth-check@2.1.1 deduped
│     └─┬ @storybook/manager-webpack4@6.5.10
│       └─┬ html-webpack-plugin@4.5.2
│         └─┬ pretty-error@2.1.2
│           └─┬ renderkid@2.0.7
│             └─┬ css-select@4.3.0
│               └── nth-check@2.1.1 deduped
├─┬ @svgr/webpack@6.0.0
│ └─┬ @svgr/plugin-svgo@6.5.0
│   └─┬ svgo@2.8.0
│     └─┬ css-select@4.3.0
│       └── nth-check@2.1.1 deduped
├─┬ html-webpack-plugin@5.5.0
│ └─┬ pretty-error@4.0.0
│   └─┬ renderkid@3.0.0
│     └─┬ css-select@4.3.0
│       └── nth-check@2.1.1 deduped
├─┬ react-native-svg-transformer@0.14.3
│ └─┬ @svgr/plugin-svgo@4.3.1
│   └─┬ svgo@1.3.2
│     └─┬ css-select@2.1.0
│       └── nth-check@1.0.2
└─┬ react-native-svg@12.4.4
  └─┬ css-select@5.1.0
    └── nth-check@2.1.1

[trjExpensify]

Cool, so once @Gonals gives this the all clear. I'll send the offer!

[hungvu193]

Cool, just let me know then I'll apply.

[trjExpensify]

@Gonals - bump on this proposal review?

[melvin-bot[bot]]

@trjExpensify, @Gonals, @Santhosh-Sellavel Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot[bot]]

@trjExpensify, @Gonals, @Santhosh-Sellavel Whoops! This issue is 2 days overdue. Let's get this updated quick!

[Gonals]

@Gonals - bump on this proposal review?

LGTM!

[hungvu193]

Cool, should I apply to upwork?

[Santhosh-Sellavel]

@Gonals What should we do about another library?

Seems we need to update another library also, react-native-svg-transformer library's dependency libraries css select uses nth-check@1.0.2

new.expensify@1.2.14-0 /Users/santhoshkumar/Documents/App

├─┬ @storybook/react@6.5.10

│ └─┬ @storybook/core@6.5.10

│   └─┬ @storybook/core-server@6.5.10

│     ├─┬ @storybook/builder-webpack4@6.5.10

│     │ └─┬ html-webpack-plugin@4.5.2

│     │   └─┬ pretty-error@2.1.2

│     │     └─┬ renderkid@2.0.7

│     │       └─┬ css-select@4.3.0

│     │         └── nth-check@2.1.1 deduped

│     └─┬ @storybook/manager-webpack4@6.5.10

│       └─┬ html-webpack-plugin@4.5.2

│         └─┬ pretty-error@2.1.2

│           └─┬ renderkid@2.0.7

│             └─┬ css-select@4.3.0

│               └── nth-check@2.1.1 deduped

├─┬ @svgr/webpack@6.0.0

│ └─┬ @svgr/plugin-svgo@6.5.0

│   └─┬ svgo@2.8.0

│     └─┬ css-select@4.3.0

│       └── nth-check@2.1.1 deduped

├─┬ html-webpack-plugin@5.5.0

│ └─┬ pretty-error@4.0.0

│   └─┬ renderkid@3.0.0

│     └─┬ css-select@4.3.0

│       └── nth-check@2.1.1 deduped

├─┬ react-native-svg-transformer@0.14.3

│ └─┬ @svgr/plugin-svgo@4.3.1

│   └─┬ svgo@1.3.2

│     └─┬ css-select@2.1.0

│       └── nth-check@1.0.2

└─┬ react-native-svg@12.4.4

  └─┬ css-select@5.1.0

    └── nth-check@2.1.1

[Gonals]

@Gonals What should we do about another library?

Seems we need to update another library also, react-native-svg-transformer library's dependency libraries css select uses nth-check@1.0.2

new.expensify@1.2.14-0 /Users/santhoshkumar/Documents/App

├─┬ @storybook/react@6.5.10

│ └─┬ @storybook/core@6.5.10

│   └─┬ @storybook/core-server@6.5.10

│     ├─┬ @storybook/builder-webpack4@6.5.10

│     │ └─┬ html-webpack-plugin@4.5.2

│     │   └─┬ pretty-error@2.1.2

│     │     └─┬ renderkid@2.0.7

│     │       └─┬ css-select@4.3.0

│     │         └── nth-check@2.1.1 deduped

│     └─┬ @storybook/manager-webpack4@6.5.10

│       └─┬ html-webpack-plugin@4.5.2

│         └─┬ pretty-error@2.1.2

│           └─┬ renderkid@2.0.7

│             └─┬ css-select@4.3.0

│               └── nth-check@2.1.1 deduped

├─┬ @svgr/webpack@6.0.0

│ └─┬ @svgr/plugin-svgo@6.5.0

│   └─┬ svgo@2.8.0

│     └─┬ css-select@4.3.0

│       └── nth-check@2.1.1 deduped

├─┬ html-webpack-plugin@5.5.0

│ └─┬ pretty-error@4.0.0

│   └─┬ renderkid@3.0.0

│     └─┬ css-select@4.3.0

│       └── nth-check@2.1.1 deduped

├─┬ react-native-svg-transformer@0.14.3

│ └─┬ @svgr/plugin-svgo@4.3.1

│   └─┬ svgo@1.3.2

│     └─┬ css-select@2.1.0

│       └── nth-check@1.0.2

└─┬ react-native-svg@12.4.4

  └─┬ css-select@5.1.0

    └── nth-check@2.1.1

I think we can update it as part of this. What do you think?

[trjExpensify]

@Santhosh-Sellavel - are we good with that? ^^

Cool, should I apply to upwork?

@hungvu193 - yes please. Looks like you haven't applied yet here.

[hungvu193]

@trjExpensify I have applied (https://www.upwork.com/ab/proposals/1587067667179589633)

[melvin-bot[bot]]

📣 @hungvu193 You have been assigned to this job by @trjExpensify! Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

[trjExpensify]

Excellent, offer sent. Assigning you the issue.

[hungvu193]

@trjExpensify I've created a PR here: https://github.com/Expensify/App/pull/12296

[Santhosh-Sellavel]

Sorry some how I missed to reply, I agree with gonals we should fix that here.

[Santhosh-Sellavel]

@hungvu193 Please handle this https://github.com/Expensify/App/issues/11797#issuecomment-1292020386 too.

[hungvu193]

@Santhosh-Sellavel Sure. It already in my PR.

[trjExpensify]

Nice speedy work!

[Santhosh-Sellavel]

@hungvu193 Please handle this #11797 (comment) too.

This is still not handled check left a comment https://github.com/Expensify/App/pull/12296#pullrequestreview-1162202174

[hungvu193]

@hungvu193 Please handle this #11797 (comment) too.

This is still not handled check left a comment #12296 (review)

let me see

[melvin-bot[bot]]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist will need to be completed before the issue can be closed:

• [ ] A regression test has been added or updated so that the same bug will not reach production again. Link to the updated test here:
• [ ] The PR that introduced the bug has been identified. Link to the PR:
• [ ] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [ ] A discussion in #contributor-plus has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [ ] Payment has been made to the issue reporter (if applicable)
• [ ] Payment has been made to the contributor that fixed the issue (if applicable)
• [ ] Payment has been made to the contributor+ that helped on the issue (if applicable)

[melvin-bot[bot]]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist will need to be completed before the issue can be closed:

• [ ] A regression test has been added or updated so that the same bug will not reach production again. Link to the updated test here:
• [ ] The PR that introduced the bug has been identified. Link to the PR:
• [ ] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [ ] A discussion in #contributor-plus has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [ ] Payment has been made to the issue reporter (if applicable)
• [ ] Payment has been made to the contributor that fixed the issue (if applicable)
• [ ] Payment has been made to the contributor+ that helped on the issue (if applicable)

[melvin-bot[bot]]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist will need to be completed before the issue can be closed:

• [x] A regression test has been added or updated so that the same bug will not reach production again. Link to the updated test here: N/A library update.
• [x] The PR that introduced the bug has been identified. Link to the PR: N/A library update.
• [x] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: N/A library update.
• [x] A discussion in #contributor-plus has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A library update.
• [x] Payment has been made to the issue reporter (if applicable) - N/A, Snyk.
• [x] Payment has been made to the contributor that fixed the issue (if applicable)
• [x] Payment has been made to the contributor+ that helped on the issue (if applicable)

[Santhosh-Sellavel]

@Gonals & @trjExpensify this should open until the regression period & payment!

[trjExpensify]

Yes, I don't know why it was closed!

[trjExpensify]

Ah, @AndrewGable you have a draft PR up to revert this do you, what's the issue? CC: @Gonals @hungvu193

[AndrewGable]

Apologies, we had a build failure on Friday and this was a PR that was flagged as possibly causing it, turns out it was something else so I've closed the revert.

[trjExpensify]

Ah neat, sounds good!

[hungvu193]

Cool, let me know if I need to update or investigate anything.

[trjExpensify]

Updated the title because Melv didn't do it. On hold for payment on the 15th after the prod deploy.

[trjExpensify]

Settled up, checklist complete.