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

Use preloading for initial requests in importer and setup wizard #3446

Merged
merged 10 commits into from
Aug 6, 2020

Conversation

yscik
Copy link
Contributor

@yscik yscik commented Jul 21, 2020

This uses the @wordpress/api-fetch library's functionality to pre-resolve some REST-API requests on the server and send the results along in the initial HTML response. With this, we don't have to show a spinner when opening the importer/exporter/setup wizard while fetching the current state (active step/ongoing job), but can display the correct screen immediately.

Changes proposed in this Pull Request

  • Add an apiFetch middleware that makes sure that preloaded responses are used only once
    • Used by importing shared/data/api-fetch-preloaded-once
  • Add preloading for getting the active job in importer/exporter
    • Adjust the response of this endpoint to return an empty job instead of 404, so that it works with preloading
  • Add preloading for getting the setup wizard state

Testing instructions

  • Open the Setup Wizard again after finishing it
    • It should display the correct state (all steps completed), without an additional network request and waiting
  • Open the Importer, start a new import. Reload the page while it's still in progress.
    • It should show the progress screen immediately

@yscik yscik requested a review from a team July 21, 2020 14:20
Copy link
Contributor

@renatho renatho left a comment

Choose a reason for hiding this comment

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

Good idea! It improved a lot the experience \o/

Just a minor comment and there are some conflicts in the branch.

assets/data-port/import.js Show resolved Hide resolved
@yscik yscik added this to the 3.5.0 milestone Jul 22, 2020
@yscik yscik force-pushed the add/apifetch-preload branch from 90c92bf to 72d5a4f Compare July 22, 2020 12:24
renatho
renatho previously approved these changes Jul 22, 2020
Copy link
Contributor

@renatho renatho left a comment

Choose a reason for hiding this comment

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

🚀

@yscik yscik force-pushed the add/exporter-api branch from 33e4c0a to ad1806d Compare July 31, 2020 16:32
@yscik yscik force-pushed the add/apifetch-preload branch from 996567b to 95fe40f Compare July 31, 2020 20:36
@yscik yscik changed the base branch from add/exporter-api to master July 31, 2020 20:36
@yscik yscik dismissed renatho’s stale review July 31, 2020 20:36

The base branch was changed.

@yscik yscik changed the title Use preloading for initial requests in Importer, Exporter, Setup Wizard Use preloading for initial requests in Importer, Setup Wizard Jul 31, 2020
@yscik
Copy link
Contributor Author

yscik commented Jul 31, 2020

Rebased and removed the exporter bit so it can be merged to master directly.

@yscik yscik requested a review from renatho July 31, 2020 20:43
@yscik yscik merged commit b7492d6 into master Aug 6, 2020
@yscik yscik deleted the add/apifetch-preload branch August 6, 2020 13:52
@donnapep donnapep changed the title Use preloading for initial requests in Importer, Setup Wizard Use preloading for initial requests in importer and setup wizard Aug 17, 2020
@jsnajdr
Copy link
Member

jsnajdr commented Dec 7, 2020

Hi @yscik 👋 I found the preloadedDataUsedOnceMiddleware utility by complete coincidence, when grepping for api-fetch in the wpcom codebase (it ships the Sensei plugin).

Just wanted to point out that "preloading just once" is what apiFetch preloading middleware now does natively, after a bugfix was merged two months ago in WordPress/gutenberg#25550.

In future versions of Sensei, when the underlying WordPress Core ships the fixed api-fetch library, you might not need the workaround any more.

@yscik
Copy link
Contributor Author

yscik commented Dec 7, 2020

Hey @jsnajdr! Thanks for the heads up, that is good news :) We support a few older versions of Wordpress in Sensei, so this might remain for a while, but we'll flag this as something that can be removed in a future release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants