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

Framework: Use WHATWG URL in place of legacy url module #19823

Merged
merged 4 commits into from Mar 12, 2020

Conversation

aduth
Copy link
Member

@aduth aduth commented Jan 22, 2020

Partially addresses: #13386
Unblocks (maybe): #19816
Closes #19629

This pull request seeks to remove usage of the Node legacy url module in favor of the WHATWG URL constructor which is native to both Node.js and modern browsers. A polyfill has been included for unsupported browsers.

The proposed advantages here are:

  • Improved future compatibility with Webpack Node.js shims
  • Reduced bundle sizes

The following bundle size changes have been observed:

  block-editor block-library
Before 346kb 387kb
After 333kb 375kb
Change -13kb (-3.7%) -12kb (-3.1%)

Testing Instructions:

Verify tests pass:

npm test

For good measure, verify affected behaviors:

@aduth aduth added Framework Issues related to broader framework topics, especially as it relates to javascript [Type] Performance Related to performance efforts [Type] Code Quality Issues or PRs that relate to code quality labels Jan 22, 2020
@aduth aduth requested a review from sgomes January 22, 2020 19:57
@gziolo gziolo added the Backport to WP Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Jan 22, 2020
Copy link
Contributor

@sgomes sgomes left a comment

Choose a reason for hiding this comment

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

Great to see Gutenberg moving away from node's url! Great work, @aduth!

I left some comments that may or may not be relevant, depending on what you expect the passed in URLs to be. The URL constructor can only produce absolute URLs, so if you're passing in a non-absolute URL you need to specify a base URL to fill in the gaps, just like you did in getResourcePath. I left comments in other parts of the code, where no base URL was provided.

@aduth
Copy link
Member Author

aduth commented Feb 7, 2020

The polyfill work landed in #19871, so this one can be rebased to eliminate that specific code.

Edit: This branch has been rebased to remove the polyfill code.

Related Core Trac ticket for upstream polyfill patch: https://core.trac.wordpress.org/ticket/49360

@aduth aduth force-pushed the update/node-url-to-native-url branch from c8c0d0a to bdddf00 Compare February 7, 2020 22:42
@aduth aduth removed the Backport to WP Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Feb 7, 2020
@aduth aduth force-pushed the update/node-url-to-native-url branch from bdddf00 to 8a6acba Compare March 6, 2020 19:20
@github-actions
Copy link

github-actions bot commented Mar 6, 2020

Size Change: -8.78 kB (1%)

Total Size: 856 kB

Filename Size Change
build/annotations/index.js 3.43 kB -2 B (0%)
build/block-directory/index.js 6.02 kB +5 B (0%)
build/block-editor/index.js 100 kB -4.21 kB (4%)
build/block-library/index.js 111 kB -4.58 kB (4%)
build/blocks/index.js 57.7 kB -2 B (0%)
build/components/index.js 191 kB +8 B (0%)
build/compose/index.js 5.76 kB +1 B
build/data-controls/index.js 1.04 kB +1 B
build/data/index.js 8.21 kB -7 B (0%)
build/date/index.js 5.37 kB +4 B (0%)
build/dom-ready/index.js 568 B -1 B
build/edit-post/index.js 91.3 kB +2 B (0%)
build/edit-widgets/index.js 4.45 kB +6 B (0%)
build/editor/index.js 43.8 kB +1 B
build/element/index.js 4.45 kB -1 B
build/escape-html/index.js 733 B -1 B
build/format-library/index.js 7.09 kB +4 B (0%)
build/html-entities/index.js 621 B -1 B
build/i18n/index.js 3.49 kB -1 B
build/list-reusable-blocks/index.js 2.99 kB -1 B
build/primitives/index.js 1.49 kB -1 B
build/priority-queue/index.js 779 B -1 B
build/redux-routine/index.js 2.84 kB +1 B
build/rich-text/index.js 14.3 kB -1 B
build/url/index.js 4 kB +1 B
build/warning/index.js 1.14 kB +1 B
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.01 kB 0 B
build/api-fetch/index.js 3.39 kB 0 B
build/autop/index.js 2.58 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 760 B 0 B
build/block-editor/style-rtl.css 10.6 kB 0 B
build/block-editor/style.css 10.6 kB 0 B
build/block-library/editor-rtl.css 7.23 kB 0 B
build/block-library/editor.css 7.24 kB 0 B
build/block-library/style-rtl.css 7.38 kB 0 B
build/block-library/style.css 7.39 kB 0 B
build/block-library/theme-rtl.css 669 B 0 B
build/block-library/theme.css 671 B 0 B
build/block-serialization-default-parser/index.js 1.65 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/components/style-rtl.css 15.6 kB 0 B
build/components/style.css 15.6 kB 0 B
build/core-data/index.js 10.6 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom/index.js 3.06 kB 0 B
build/edit-post/style-rtl.css 8.64 kB 0 B
build/edit-post/style.css 8.64 kB 0 B
build/edit-site/index.js 4.64 kB 0 B
build/edit-site/style-rtl.css 2.48 kB 0 B
build/edit-site/style.css 2.48 kB 0 B
build/edit-widgets/style-rtl.css 2.58 kB 0 B
build/edit-widgets/style.css 2.58 kB 0 B
build/editor/editor-styles-rtl.css 381 B 0 B
build/editor/editor-styles.css 382 B 0 B
build/editor/style-rtl.css 3.98 kB 0 B
build/editor/style.css 3.97 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 1.92 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.3 kB 0 B
build/keycodes/index.js 1.68 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 4.85 kB 0 B
build/notices/index.js 1.57 kB 0 B
build/nux/index.js 3.01 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.54 kB 0 B
build/server-side-render/index.js 2.55 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/viewport/index.js 1.61 kB 0 B
build/wordcount/index.js 1.18 kB 0 B

compressed-size-action

@aduth aduth force-pushed the update/node-url-to-native-url branch from 8a6acba to 26a0dbc Compare March 11, 2020 19:30
@aduth aduth merged commit 3b63be6 into master Mar 12, 2020
@aduth aduth deleted the update/node-url-to-native-url branch March 12, 2020 19:59
@github-actions github-actions bot added this to the Gutenberg 7.8 milestone Mar 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Framework Issues related to broader framework topics, especially as it relates to javascript [Type] Code Quality Issues or PRs that relate to code quality [Type] Performance Related to performance efforts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LinkControl - improve "valid entry" matching (not always a URL!)
3 participants