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

Remove custom rest_preload_api_request fork #12626

Closed
wants to merge 4 commits into from

Conversation

spacedmonkey
Copy link
Contributor

@spacedmonkey spacedmonkey commented Nov 4, 2022

Context

Summary

Remove custom rest_preload_api_request as not needed with WP 5.9 as min.

Relevant Technical Choices

To-do

User-facing changes

Testing Instructions

  • This is a non-user-facing change and requires no QA

This PR can be tested by following these steps:

Reviews

Does this PR have a security-related impact?

Does this PR change what data or activity we track or use?

Does this PR have a legal-related impact?

Checklist

  • This PR addresses an existing issue and I have linked this PR to it in ZenHub
  • I have tested this code to the best of my abilities
  • I have verified accessibility to the best of my abilities (docs)
  • I have verified i18n and l10n (translation, right-to-left layout) to the best of my abilities
  • This code is covered by automated tests (unit, integration, and/or e2e) to verify it works as intended (docs)
  • I have added documentation where necessary
  • I have added a matching Type: XYZ label to the PR

Fixes #12622

@spacedmonkey spacedmonkey self-assigned this Nov 4, 2022
@spacedmonkey spacedmonkey added Type: Infrastructure Changes impacting testing infrastructure or build tooling PHP Pull requests that update PHP code Pod: WP labels Nov 4, 2022
@swissspidy swissspidy added Type: Code Quality Things that need a refactor, rewrite or just some good old developer ❤️ and removed Type: Infrastructure Changes impacting testing infrastructure or build tooling PHP Pull requests that update PHP code Pod: WP labels Nov 4, 2022
@googleforcreators-bot
Copy link
Collaborator

googleforcreators-bot commented Nov 4, 2022

Plugin builds for d5c4484 are ready 🛎️!

@github-actions
Copy link
Contributor

github-actions bot commented Nov 4, 2022

Size Change: 0 B

Total Size: 2.72 MB

ℹ️ View Unchanged
Filename Size
assets/css/carousel-view-rtl.css 702 B
assets/css/carousel-view.css 701 B
assets/css/web-stories-block-rtl.css 4.51 kB
assets/css/web-stories-block.css 4.56 kB
assets/css/web-stories-embed-rtl.css 318 B
assets/css/web-stories-embed.css 317 B
assets/css/web-stories-list-styles-rtl.css 2.35 kB
assets/css/web-stories-list-styles.css 2.38 kB
assets/css/web-stories-theme-style-twentyeleven-rtl.css 102 B
assets/css/web-stories-theme-style-twentyeleven.css 102 B
assets/css/web-stories-theme-style-twentyfifteen-rtl.css 251 B
assets/css/web-stories-theme-style-twentyfifteen.css 251 B
assets/css/web-stories-theme-style-twentyfourteen-rtl.css 287 B
assets/css/web-stories-theme-style-twentyfourteen.css 287 B
assets/css/web-stories-theme-style-twentyseventeen-rtl.css 288 B
assets/css/web-stories-theme-style-twentyseventeen.css 288 B
assets/css/web-stories-theme-style-twentysixteen-rtl.css 224 B
assets/css/web-stories-theme-style-twentysixteen.css 224 B
assets/css/web-stories-theme-style-twentyten-rtl.css 143 B
assets/css/web-stories-theme-style-twentyten.css 143 B
assets/css/web-stories-theme-style-twentytwelve-rtl.css 256 B
assets/css/web-stories-theme-style-twentytwelve.css 256 B
assets/css/web-stories-theme-style-twentytwenty-rtl.css 86 B
assets/css/web-stories-theme-style-twentytwenty.css 86 B
assets/css/web-stories-theme-style-twentytwentyone-rtl.css 326 B
assets/css/web-stories-theme-style-twentytwentyone.css 326 B
assets/css/web-stories-widget-rtl.css 482 B
assets/css/web-stories-widget.css 482 B
assets/css/wp-dashboard-rtl.css 657 B
assets/css/wp-dashboard.css 659 B
assets/css/wp-story-editor-rtl.css 769 B
assets/css/wp-story-editor.css 771 B
assets/js/2475.js 7.78 kB
assets/js/3371.js 194 kB
assets/js/4422.js 49.3 kB
assets/js/6071.js 92.9 kB
assets/js/9162.js 36.5 kB
assets/js/9750.js 12.8 kB
assets/js/carousel-view.js 3.5 kB
assets/js/chunk-colorthief.js 2.64 kB
assets/js/chunk-ffmpeg.js 5.88 kB
assets/js/chunk-html-to-image.js 4.51 kB
assets/js/chunk-opentype.js 96 B
assets/js/chunk-react-calendar.js 12.5 kB
assets/js/chunk-react-color.js 44.3 kB
assets/js/chunk-selfie-segmentation.js 12.5 kB
assets/js/chunk-web-animations-js.js 14.6 kB
assets/js/chunk-web-stories-template-0-metaData.js 546 B
assets/js/chunk-web-stories-template-0.js 11.4 kB
assets/js/chunk-web-stories-template-1-metaData.js 540 B
assets/js/chunk-web-stories-template-1.js 9.6 kB
assets/js/chunk-web-stories-template-10-metaData.js 532 B
assets/js/chunk-web-stories-template-10.js 7.36 kB
assets/js/chunk-web-stories-template-11-metaData.js 539 B
assets/js/chunk-web-stories-template-11.js 9.07 kB
assets/js/chunk-web-stories-template-12-metaData.js 496 B
assets/js/chunk-web-stories-template-12.js 9.68 kB
assets/js/chunk-web-stories-template-13-metaData.js 525 B
assets/js/chunk-web-stories-template-13.js 7.39 kB
assets/js/chunk-web-stories-template-14-metaData.js 582 B
assets/js/chunk-web-stories-template-14.js 7.37 kB
assets/js/chunk-web-stories-template-15-metaData.js 544 B
assets/js/chunk-web-stories-template-15.js 9 kB
assets/js/chunk-web-stories-template-16-metaData.js 588 B
assets/js/chunk-web-stories-template-16.js 10.8 kB
assets/js/chunk-web-stories-template-17-metaData.js 540 B
assets/js/chunk-web-stories-template-17.js 9.2 kB
assets/js/chunk-web-stories-template-18-metaData.js 587 B
assets/js/chunk-web-stories-template-18.js 9.9 kB
assets/js/chunk-web-stories-template-19-metaData.js 501 B
assets/js/chunk-web-stories-template-19.js 10.8 kB
assets/js/chunk-web-stories-template-2-metaData.js 586 B
assets/js/chunk-web-stories-template-2.js 9.29 kB
assets/js/chunk-web-stories-template-20-metaData.js 548 B
assets/js/chunk-web-stories-template-20.js 8.99 kB
assets/js/chunk-web-stories-template-21-metaData.js 536 B
assets/js/chunk-web-stories-template-21.js 9.85 kB
assets/js/chunk-web-stories-template-22-metaData.js 525 B
assets/js/chunk-web-stories-template-22.js 7.83 kB
assets/js/chunk-web-stories-template-23-metaData.js 604 B
assets/js/chunk-web-stories-template-23.js 7.47 kB
assets/js/chunk-web-stories-template-24-metaData.js 517 B
assets/js/chunk-web-stories-template-24.js 11.7 kB
assets/js/chunk-web-stories-template-25-metaData.js 543 B
assets/js/chunk-web-stories-template-25.js 7.06 kB
assets/js/chunk-web-stories-template-26-metaData.js 600 B
assets/js/chunk-web-stories-template-26.js 7.26 kB
assets/js/chunk-web-stories-template-27-metaData.js 542 B
assets/js/chunk-web-stories-template-27.js 7.8 kB
assets/js/chunk-web-stories-template-28-metaData.js 532 B
assets/js/chunk-web-stories-template-28.js 9.05 kB
assets/js/chunk-web-stories-template-29-metaData.js 561 B
assets/js/chunk-web-stories-template-29.js 9.24 kB
assets/js/chunk-web-stories-template-3-metaData.js 539 B
assets/js/chunk-web-stories-template-3.js 8.39 kB
assets/js/chunk-web-stories-template-30-metaData.js 576 B
assets/js/chunk-web-stories-template-30.js 7.88 kB
assets/js/chunk-web-stories-template-31-metaData.js 503 B
assets/js/chunk-web-stories-template-31.js 10.3 kB
assets/js/chunk-web-stories-template-32-metaData.js 552 B
assets/js/chunk-web-stories-template-32.js 13.3 kB
assets/js/chunk-web-stories-template-33-metaData.js 491 B
assets/js/chunk-web-stories-template-33.js 9.06 kB
assets/js/chunk-web-stories-template-34-metaData.js 570 B
assets/js/chunk-web-stories-template-34.js 7.57 kB
assets/js/chunk-web-stories-template-35-metaData.js 565 B
assets/js/chunk-web-stories-template-35.js 8.9 kB
assets/js/chunk-web-stories-template-36-metaData.js 575 B
assets/js/chunk-web-stories-template-36.js 12.7 kB
assets/js/chunk-web-stories-template-37-metaData.js 529 B
assets/js/chunk-web-stories-template-37.js 6.71 kB
assets/js/chunk-web-stories-template-38-metaData.js 572 B
assets/js/chunk-web-stories-template-38.js 7.92 kB
assets/js/chunk-web-stories-template-39-metaData.js 589 B
assets/js/chunk-web-stories-template-39.js 8.07 kB
assets/js/chunk-web-stories-template-4-metaData.js 564 B
assets/js/chunk-web-stories-template-4.js 12.7 kB
assets/js/chunk-web-stories-template-40-metaData.js 556 B
assets/js/chunk-web-stories-template-40.js 10.2 kB
assets/js/chunk-web-stories-template-41-metaData.js 573 B
assets/js/chunk-web-stories-template-41.js 7.74 kB
assets/js/chunk-web-stories-template-42-metaData.js 521 B
assets/js/chunk-web-stories-template-42.js 6.99 kB
assets/js/chunk-web-stories-template-43-metaData.js 557 B
assets/js/chunk-web-stories-template-43.js 8.75 kB
assets/js/chunk-web-stories-template-44-metaData.js 583 B
assets/js/chunk-web-stories-template-44.js 11.1 kB
assets/js/chunk-web-stories-template-45-metaData.js 565 B
assets/js/chunk-web-stories-template-45.js 7.5 kB
assets/js/chunk-web-stories-template-46-metaData.js 531 B
assets/js/chunk-web-stories-template-46.js 5.21 kB
assets/js/chunk-web-stories-template-47-metaData.js 591 B
assets/js/chunk-web-stories-template-47.js 9.41 kB
assets/js/chunk-web-stories-template-48-metaData.js 556 B
assets/js/chunk-web-stories-template-48.js 9.08 kB
assets/js/chunk-web-stories-template-49-metaData.js 518 B
assets/js/chunk-web-stories-template-49.js 9.69 kB
assets/js/chunk-web-stories-template-5-metaData.js 556 B
assets/js/chunk-web-stories-template-5.js 9.91 kB
assets/js/chunk-web-stories-template-50-metaData.js 503 B
assets/js/chunk-web-stories-template-50.js 9.13 kB
assets/js/chunk-web-stories-template-51-metaData.js 526 B
assets/js/chunk-web-stories-template-51.js 10.3 kB
assets/js/chunk-web-stories-template-52-metaData.js 601 B
assets/js/chunk-web-stories-template-52.js 10.3 kB
assets/js/chunk-web-stories-template-53-metaData.js 551 B
assets/js/chunk-web-stories-template-53.js 5.78 kB
assets/js/chunk-web-stories-template-54-metaData.js 547 B
assets/js/chunk-web-stories-template-54.js 7.66 kB
assets/js/chunk-web-stories-template-55-metaData.js 574 B
assets/js/chunk-web-stories-template-55.js 7.12 kB
assets/js/chunk-web-stories-template-56-metaData.js 542 B
assets/js/chunk-web-stories-template-56.js 9.85 kB
assets/js/chunk-web-stories-template-57-metaData.js 528 B
assets/js/chunk-web-stories-template-57.js 14.9 kB
assets/js/chunk-web-stories-template-58-metaData.js 554 B
assets/js/chunk-web-stories-template-58.js 5.74 kB
assets/js/chunk-web-stories-template-59-metaData.js 590 B
assets/js/chunk-web-stories-template-59.js 8.95 kB
assets/js/chunk-web-stories-template-6-metaData.js 568 B
assets/js/chunk-web-stories-template-6.js 7.05 kB
assets/js/chunk-web-stories-template-60-metaData.js 509 B
assets/js/chunk-web-stories-template-60.js 9.51 kB
assets/js/chunk-web-stories-template-7-metaData.js 569 B
assets/js/chunk-web-stories-template-7.js 7.45 kB
assets/js/chunk-web-stories-template-8-metaData.js 569 B
assets/js/chunk-web-stories-template-8.js 8.9 kB
assets/js/chunk-web-stories-template-9-metaData.js 580 B
assets/js/chunk-web-stories-template-9.js 8.45 kB
assets/js/chunk-web-stories-templates.js 1.17 kB
assets/js/chunk-web-stories-textset-0.js 5.04 kB
assets/js/chunk-web-stories-textset-1.js 6.63 kB
assets/js/chunk-web-stories-textset-2.js 7.62 kB
assets/js/chunk-web-stories-textset-3.js 15 kB
assets/js/chunk-web-stories-textset-4.js 4.14 kB
assets/js/chunk-web-stories-textset-5.js 5.45 kB
assets/js/chunk-web-stories-textset-6.js 5.25 kB
assets/js/chunk-web-stories-textset-7.js 10.1 kB
assets/js/generateBlurhash.worker.worker.js 1.1 kB
assets/js/imgareaselect.js 3.77 kB
assets/js/lightbox.js 550 B
assets/js/tinymce-button.js 2.85 kB
assets/js/web-stories-activation-notice.js 27.1 kB
assets/js/web-stories-block.js 22.7 kB
assets/js/web-stories-embed.js 20 B
assets/js/web-stories-widget.js 587 B
assets/js/wp-dashboard.js 64.2 kB
assets/js/wp-story-editor.js 1.44 MB

compressed-size-action

@swissspidy swissspidy added the Status: Blocked On hold for the time being label Nov 7, 2022
@swissspidy swissspidy changed the title Remove custom rest_preload_api_request. Remove custom rest_preload_api_request fork Nov 9, 2022
@swissspidy swissspidy removed the Status: Blocked On hold for the time being label Nov 9, 2022
@swissspidy swissspidy self-requested a review November 9, 2022 14:55
@swissspidy
Copy link
Collaborator

So the e2e tests seem to be failing and from what I can see, the preloading is not actually working anymore.

Maybe still an issue with how core's function handles embeds or something.

@spacedmonkey
Copy link
Contributor Author

@swissspidy Sadly, it is seems we were too early on this one. There is another tweak that was made to rest_preload_api_request function that is breaking preload. These lines.

$request = new WP_REST_Request( $method, untrailingslashit( $path_parts['path'] ) );

@swissspidy
Copy link
Collaborator

Ah yes, the trailing slashes issue.

Wasn't that supposed to be fixed by https://core.trac.wordpress.org/ticket/51636?

@spacedmonkey
Copy link
Contributor Author

untrailingslashit

Sadly it seems like it was not :( We can't use that function from core then.

@spacedmonkey
Copy link
Contributor Author

untrailingslashit

Sadly it seems like it was not :( We can't use that function from core then.

I will report upstream.

@swissspidy swissspidy deleted the fix/remove-rest_preload_api_request branch November 9, 2022 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Code Quality Things that need a refactor, rewrite or just some good old developer ❤️
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove custom rest_preload_api_request function
3 participants