Skip to content
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

chore(deps): #4267 upgrade lodash to latest #4284

Merged
merged 4 commits into from
Oct 8, 2018
Merged

chore(deps): #4267 upgrade lodash to latest #4284

merged 4 commits into from
Oct 8, 2018

Conversation

Stephanemw
Copy link
Contributor

@Stephanemw Stephanemw commented Oct 4, 2018

Issue: Fix for #4267

What I did

Upgraded lodash to ^4.17.11, updated imports and mocks (from lodash.xxx to lodash/xxx)

How to test

yarn test --core

@storybook-safe-bot
Copy link
Contributor

Fails
🚫

PR is not labeled with one of: ["cleanup","BREAKING CHANGE","feature request","bug","documentation","maintenance","dependencies:update","dependencies","other"]

Generated by 🚫 dangerJS

@wuweiweiwu wuweiweiwu added the maintenance User-facing maintenance tasks label Oct 6, 2018
Copy link
Member

@wuweiweiwu wuweiweiwu left a comment

Choose a reason for hiding this comment

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

LGTM!

@igor-dv
Copy link
Member

igor-dv commented Oct 6, 2018

As far as I remember, when we tried the es6 version last time, it broke the users' code in Storyshots, because they needed to configure jest to transpile lodash from the node_modules.

@Stephanemw
Copy link
Contributor Author

Stephanemw commented Oct 7, 2018

As far as I remember, when we tried the es6 version last time, it broke the users' code in Storyshots, because they needed to configure jest to transpile lodash from the node_modules.

Do we have a testcase in the examples that exhibits this breakage? Otherwise, I suppose we could always request the various 2-year old lodash.xxxx packages be upgraded to address that CVE... Looks like the reason for those packages not being updated is the upcoming Lodash v5 getting rid of them https://github.com/lodash/lodash/wiki/Roadmap (point #2)

@igor-dv
Copy link
Member

igor-dv commented Oct 8, 2018

Do we have a testcase in the examples that exhibits this breakage?

I've checked it now, looks like they have a build process. And the problem we had before was with the lodash-es package.

Update from storybook/master
…:Stephanemw/storybook into build/4267_lodash_upgrade
@codecov
Copy link

codecov bot commented Oct 8, 2018

Codecov Report

Merging #4284 into master will increase coverage by 0.31%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4284      +/-   ##
==========================================
+ Coverage   39.87%   40.18%   +0.31%     
==========================================
  Files         504      510       +6     
  Lines        5908     5942      +34     
  Branches      793      792       -1     
==========================================
+ Hits         2356     2388      +32     
- Misses       3168     3169       +1     
- Partials      384      385       +1
Impacted Files Coverage Δ
lib/ui/src/modules/ui/libs/filters.js 100% <ø> (ø) ⬆️
addons/actions/src/preview/withActions.js 25.8% <ø> (ø) ⬆️
lib/ui/src/modules/ui/containers/layout.js 100% <ø> (ø) ⬆️
lib/ui/src/modules/ui/containers/header.js 88.88% <ø> (ø) ⬆️
lib/components/src/layout/desktop.js 73.68% <ø> (ø) ⬆️
...i/src/modules/ui/components/stories_panel/index.js 100% <ø> (ø) ⬆️
lib/ui/src/modules/api/actions/api.js 100% <ø> (ø) ⬆️
lib/ui/src/modules/shortcuts/actions/shortcuts.js 40.62% <ø> (ø) ⬆️
app/react/src/client/preview/element_check.js 100% <ø> (ø) ⬆️
...modules/ui/components/stories_panel/text_filter.js 100% <ø> (ø) ⬆️
... and 12 more

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 f7ccb82...11bf9f1. Read the comment docs.

@igor-dv igor-dv merged commit be3ab29 into storybookjs:master Oct 8, 2018
@Stephanemw Stephanemw deleted the build/4267_lodash_upgrade branch October 11, 2018 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants