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.

Sign up for GitHub

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

Jump to bottom

Update lodash to 4.17.10 #13651

Merged
merged 1 commit into from
Feb 4, 2019
Merged

Update lodash to 4.17.10 #13651

merged 1 commit into from
Feb 4, 2019

Conversation

jorgefilipecosta
Copy link
Member

Description

Requesting lodash 4.17.5 from unpkg was systematically failing making building the zip of the plugin impossible.

https://unpkg.com/[email protected]/umd/react-dom.production.min.js
 > vendor/react-dom.min.713f0afa.js ... done!
https://unpkg.com/[email protected]/min/moment.min.js
 > vendor/moment.min.42810f9e.js ... done!
https://unpkg.com/[email protected]/lodash.min.js
 > vendor/lodash.min.59550321.js ... 
HTTP 500

Requesting 4.17.10 was working perfectly fine. There are many references to 4.17.10 across our packages so I think it is fine to update the version downloaded from unpkg.

Meanwhile after many tries to download https://unpkg.com/[email protected]/lodash.min.js and getting error 500, it started working.
I guess older versions are not widely distributed in the CDN and this update is an improvement to avoid problems while building the zip in the future.

How has this been tested?

I checked Gutenberg works normally (smoke testing).
I checked building Gutenberg zip works correctly ./bin/build-plugin-zip.sh
I checked I can download https://unpkg.com/[email protected]/lodash.min.js without any problem.

@youknowriad
Copy link
Contributor

I don't feel strongly but was thinking maybe we should get rid of all the vendors handling from the plugin now that the minimum version is 5.0.

If we need to update one of these, we could override the script like we do for the packages, but otherwise it's not necessary to maintain in the plugin anymore.

what do you think?

cc @aduth as you're working on the PHP removals.

gziolo
gziolo approved these changes Feb 4, 2019
View reviewed changes
Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

It looks like something that was missed down the road.

@gziolo
Copy link
Member

gziolo commented Feb 4, 2019

I'm sure we will have to override React pretty soon to start using hooks. Saying that I think we should remove all overrides but leave the room to be able to do so in the near future. Maybe, we simply wait for React 16.8 and bump it for plugin and remove all overrides at the same time :)

@gziolo gziolo added this to the 5.0 (Gutenberg) milestone Feb 4, 2019
@gziolo gziolo added the [Type] Build Tooling Issues or PRs related to build tooling label Feb 4, 2019
@gziolo gziolo merged commit 317b219 into master Feb 4, 2019
@gziolo gziolo deleted the update/lodash-to-4.17.10 branch February 4, 2019 11:27
@aduth
Copy link
Member

aduth commented Feb 4, 2019

I don't feel strongly but was thinking maybe we should get rid of all the vendors handling from the plugin now that the minimum version is 5.0.

See #13573

@aduth
Copy link
Member

aduth commented Feb 4, 2019 •
edited
Loading

FYI, Core uses [email protected], which was already (and still) different from what we use in Gutenberg.

https://github.com/WordPress/wordpress-develop/blob/3e19129dc94b6c07239bdd688b74de1bc981d7a2/package.json#L103

daniloercoli added a commit that referenced this pull request Feb 5, 2019
@daniloercoli
Merge branch 'master' of https://github.com/WordPress/gutenberg into …
70836f1 
…rnmobile/372-use-RichText-on-Title-block

* 'master' of https://github.com/WordPress/gutenberg: (22 commits)
  Make the modal title styling consistent (#13669)
  Disable navigation block for text mode. (#12185)
  Fix: Linting problem in modal example code (#13671)
  Add myself as a code owner to the annotations (#13672)
  Add more reviewers to CODEOWNERS.md file (#13667)
  Plugin: Remove jQuery heartbeat-to-hooks proxying (#13576)
  Workflow: Add repository CODEOWNERS file (#13604)
  Add a mobile minimum size for form fields (#13639)
  Update edit-save documentation  (#13578)
  Alt image setting (#13631)
  Fix: Allow years lower than 1970 in DateTime component. (#13602)
  Using addQueryArgs to generate Manage All Reusable Blocks link (#13653)
  Bump plugin version to 5.0.0-rc.1 (#13652)
  Update lodash to 4.17.10 (#13651)
  Refreshed PR (#9469)
  Set default values of the width and height input fields according to the actual image dimensions (#7687)
  12647 fix css color picker (#12747)
  Remove "we" from messages (#13644)
  Fix: Font size picker max width on mobile (#13264)
  Fix/issue 12501 menu item aria label
  ...
This was referenced Feb 18, 2019
Merged
Merged
youknowriad pushed a commit that referenced this pull request Mar 6, 2019
@jorgefilipecosta @youknowriad
Update lodash to 4.17.10 (#13651)
a396693
youknowriad pushed a commit that referenced this pull request Mar 6, 2019
@jorgefilipecosta @youknowriad
Update lodash to 4.17.10 (#13651)
084a1ac
This was referenced Apr 30, 2020
Closed
Closed
Merged
Merged
Merged
Closed
Closed
Closed
Closed
Merged
Closed
Merged
Closed
This was referenced May 1, 2020
Merged
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gziolo gziolo gziolo approved these changes

Assignees
No one assigned
Labels
[Type] Build Tooling Issues or PRs related to build tooling
Projects
None yet
Milestone
5.1 (Gutenberg)
Development

Successfully merging this pull request may close these issues.
4 participants
@jorgefilipecosta @youknowriad @gziolo @aduth