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

Upgrade showdown and do some lockfile gardening #21862

Merged
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 sgomes added the [Type] Performance Related to performance efforts label Apr 24, 2020
@sgomes sgomes requested review from gziolo and aduth April 24, 2020 10:14
@gziolo
Copy link
Member

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 bot commented Apr 24, 2020

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 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 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 :)

@sgomes
Copy link
Contributor Author

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 commented Apr 24, 2020

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

@sgomes sgomes requested a review from gziolo April 24, 2020 11:51
@sgomes
Copy link
Contributor Author

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 commented Apr 24, 2020

Looks good 👍

@sgomes
Copy link
Contributor Author

sgomes commented Apr 24, 2020

Thanks again, @gziolo!

@sgomes sgomes merged commit b437b67 into master Apr 24, 2020
@sgomes sgomes deleted the update/upgrade-showdown-and-gardening branch April 24, 2020 14:04
@github-actions github-actions bot added this to the Gutenberg 8.0 milestone Apr 24, 2020
@aduth
Copy link
Member

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 commented Apr 26, 2020

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 commented Apr 27, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Performance Related to performance efforts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants