Skip to content

/ gutenberg

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

Jump to bottom

Upgrade showdown and do some lockfile gardening #21862

Merged
merged 3 commits into from Apr 24, 2020
Merged

Upgrade showdown and do some lockfile gardening #21862

merged 3 commits into from Apr 24, 2020

Conversation

@sgomes
Copy link
Contributor

@sgomes sgomes commented Apr 24, 2020

Description

The showdown dependency was upgraded to the latest version, 1.9.1. This is a minor version, so it should be backwards compatible.
The 1.9.x range is significantly smaller than the currently used version, 1.8.6, by about 20KB uncompressed, or 9KB gzipped.

In addition, I performed some gardening on the lockfile, reducing duplicate dependencies by reusing the same version where the ranges allow.

This PR as a whole should save around 23KB uncompressed (9KB gzipped) in the blocks chunk, and a much tinier 3 KB uncompressed (860B) in the components chunk, without any code changes. I guess the GitHub action will tell!

How has this been tested?

Ran build, ran unit tests, performed ad-hoc testing of editor.

Types of changes

Dependency upgrades and lockfile management only.

Checklist:

  • My code is tested.
@sgomes
Upgrade showdown and do some lockfile gardening
89fb10f
@sgomes sgomes requested review from gziolo and aduth Apr 24, 2020
@gziolo
Copy link
Member

@gziolo gziolo commented Apr 24, 2020

Should we also update showdown in @wordpress/blocks to ensure all 3rd party projects have the most performant version?

It's still at "showdown": "^1.8.6",

@github-actions
Copy link

@github-actions github-actions bot commented Apr 24, 2020 •
edited

Size Change: -10.4 kB (1%)

Total Size: 835 kB

Filename Size Change
build/block-editor/index.js 105 kB +73 B (0%)
build/block-editor/style-rtl.css 10 kB -144 B (1%)
build/block-editor/style.css 10 kB -139 B (1%)
build/block-library/editor-rtl.css 7.05 kB -81 B (1%)
build/block-library/editor.css 7.05 kB -78 B (1%)
build/block-library/index.js 112 kB -95 B (0%)
build/block-library/style-rtl.css 7.14 kB -46 B (0%)
build/block-library/style.css 7.14 kB -49 B (0%)
build/blocks/index.js 48.1 kB -9.58 kB (19%) 👏
build/components/index.js 198 kB -231 B (0%)
build/edit-post/style-rtl.css 12.4 kB -13 B (0%)
build/edit-post/style.css 12.4 kB -13 B (0%)
build/edit-site/style-rtl.css 5.24 kB -8 B (0%)
build/edit-site/style.css 5.24 kB -8 B (0%)
build/editor/index.js 43.4 kB +2 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.02 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/api-fetch/index.js 4.08 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.24 kB 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 761 B 0 B
build/block-library/theme-rtl.css 683 B 0 B
build/block-library/theme.css 685 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/components/style-rtl.css 16.9 kB 0 B
build/components/style.css 16.9 kB 0 B
build/compose/index.js 6.66 kB 0 B
build/core-data/index.js 11.4 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.43 kB 0 B
build/date/index.js 5.47 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.1 kB 0 B
build/edit-navigation/index.js 3.54 kB 0 B
build/edit-navigation/style-rtl.css 485 B 0 B
build/edit-navigation/style.css 485 B 0 B
build/edit-post/index.js 28.2 kB 0 B
build/edit-site/index.js 10.8 kB 0 B
build/edit-widgets/index.js 8.33 kB 0 B
build/edit-widgets/style-rtl.css 5 kB 0 B
build/edit-widgets/style.css 5 kB 0 B
build/editor/editor-styles-rtl.css 428 B 0 B
build/editor/editor-styles.css 431 B 0 B
build/editor/style-rtl.css 3.27 kB 0 B
build/editor/style.css 3.27 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.32 kB 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 711 B 0 B
build/keyboard-shortcuts/index.js 2.51 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 5.29 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.67 kB 0 B
build/primitives/index.js 1.49 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/rich-text/index.js 14.8 kB 0 B
build/server-side-render/index.js 2.67 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.02 kB 0 B
build/viewport/index.js 1.84 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@sgomes
Copy link
Contributor Author

@sgomes sgomes commented Apr 24, 2020

Should we also update showdown in @wordpress/blocks to ensure all 3rd party projects have the most performant version?

It's still at "showdown": "^1.8.6",

@gziolo Do you mean updating the version range from ^1.8.6 to ^1.9.1? Both should pick up 1.9.1 on install, but the latter will force the new version.

@gziolo
Copy link
Member

@gziolo gziolo commented Apr 24, 2020

Should we also update showdown in @wordpress/blocks to ensure all 3rd party projects have the most performant version?
It's still at "showdown": "^1.8.6",

@gziolo Do you mean updating the version range from ^1.8.6 to ^1.9.1? Both should pick up 1.9.1 on install, but the latter will force the new version.

Yes, it will also ensure we publish a new patch version of the package (well, it would happend regardless for sure). It's definitely nice to have but it might be also a good practice to follow :)

@gziolo
gziolo approved these changes Apr 24, 2020
View changes
@sgomes
Force showdown 1.9.1 in blocks
fb78bde
@sgomes sgomes requested review from ellatrix and youknowriad as code owners Apr 24, 2020
@sgomes
Copy link
Contributor Author

@sgomes sgomes commented Apr 24, 2020

@gziolo Do you mean updating the version range from ^1.8.6 to ^1.9.1? Both should pick up 1.9.1 on install, but the latter will force the new version.

Yes, it will also ensure we publish a new patch version of the package (well, it would happend regardless for sure). It's definitely nice to have but it might be also a good practice to follow :)

Sounds good, done! 🙂

@gziolo
Copy link
Member

@gziolo gziolo commented Apr 24, 2020

All good, feel free to merge when Travis is green.

@sgomes
Run npm i to appease "Build artifacts" CI check
1eb17e3
@sgomes sgomes requested a review from gziolo Apr 24, 2020
@sgomes
Copy link
Contributor Author

@sgomes sgomes commented Apr 24, 2020

@gziolo I think it's best if you take another look after the last commit, since there are now small unexpected changes in other packages. Travis isn't complaining any more about how running npm i produces a different lockfile, though.

Thank you!

@gziolo
Copy link
Member

@gziolo gziolo commented Apr 24, 2020

Looks good 👍

@sgomes
Copy link
Contributor Author

@sgomes sgomes commented Apr 24, 2020

Thanks again, @gziolo!

@sgomes sgomes merged commit b437b67 into master Apr 24, 2020
3 checks passed
3 checks passed
@github-actions
build
Details
@github-actions
pull-request-automation
Details
@travis-ci
Travis CI - Pull Request Build Passed
Details
@sgomes sgomes deleted the update/upgrade-showdown-and-gardening branch Apr 24, 2020
@github-actions github-actions bot added this to the Gutenberg 8.0 milestone Apr 24, 2020
@aduth
Copy link
Member

@aduth aduth commented Apr 26, 2020

In addition, I performed some gardening on the lockfile, reducing duplicate dependencies by reusing the same version where the ranges allow.

Can you elaborate more on what was done here, and why? I'm encountering some issues in my pull request #21868 where I'm seeing "Build artifacts" that I'm unable to reproduce locally. I see almost all recent builds are failing as well. These changes seem particularly suspect.

@aduth
Copy link
Member

@aduth aduth commented Apr 26, 2020 •
edited

I'm encountering some issues in my pull request #21868 where I'm seeing "Build artifacts" that I'm unable to reproduce locally.

I was able to reproduce locally after removing local node_modules. Hoping c37ce38 will resolve it. Not sure it's actually related to anything here.

@sgomes
Copy link
Contributor Author

@sgomes sgomes commented Apr 27, 2020 •
edited

I was able to reproduce locally after removing local node_modules. Hoping c37ce38 will resolve it. Not sure it's actually related to anything here.

@aduth Right, that does sound unrelated, as far as I can tell.

For reference, what I did was an npm dedupe, followed by a lot of manual editing, since npm dedupe likes to mangle the version and resolved fields together, for some reason.

After this, an npm i is needed too, otherwise the "Build artifacts" CI check will complain (and since npm dedupe often leaves the lockfile in a bad state, yay bugs). Since you're doing an npm i, however, you need to look out for changed packages and make sure there were no unexpected changes.

Managing monorepos with npm can be a pain.

@sgomes sgomes mentioned this pull request Apr 29, 2020
Open
@dependabot dependabot bot mentioned this pull request Mar 18, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aduth aduth

@ellatrix ellatrix

@youknowriad youknowriad

@gziolo gziolo

Assignees
No one assigned
Labels
[Type] Performance
Projects
None yet
Milestone
Gutenberg 8.0
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
@sgomes @gziolo @aduth