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

Add new required props to RouterProvider #53352

Merged
merged 3 commits into from
Dec 2, 2024
Merged

Conversation

Aljullu
Copy link
Contributor

@Aljullu Aljullu commented Dec 2, 2024

Changes proposed in this Pull Request:

This PR fixes an issue between Customize Your Store and the most recent nightly of Gutenberg. The changes in the API were introduced in WordPress/gutenberg#67199 (thanks @gigitux for pointing it out!).

This PR is targeting release/9.4 because I think we should do a Point Release Request. In theory, the breaking changes won't be released until Gutenberg 19.9 (scheduled for December 18th), so we could target WC 9.5 (scheduled for December 16th). But I think it's risky because if the WC release is delayed just a couple of days, there would be an incompatibility between Gutenberg and WooCommerce. So I think it's safer to target WC 9.4.

How to test the changes in this Pull Request:

  1. Install the latest Gutenberg Nightly.
  2. If needed, restart the Customize Your Store flow by installing the WooCommerce Beta Tester plugin and going to wp-admin/tools.php?page=woocommerce-admin-test-helper > Tools > "Reset Customize Your Store".
  3. Go to WooCommerce > Customize Your Store. Verify there are no errors displayed on the screen.
  4. Smoke test the CYS functionality: change the logo, fonts, colors, some patterns, etc. and save it.
  5. Verify at no moment you see an error and all redirects between screens work as expected.

@Aljullu Aljullu added the focus: customize-your-store Issues related to the Customize Your Store onboarding flow. label Dec 2, 2024
@Aljullu Aljullu self-assigned this Dec 2, 2024
@github-actions github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Dec 2, 2024
Copy link
Contributor

github-actions bot commented Dec 2, 2024

Test using WordPress Playground

The changes in this pull request can be previewed and tested using a WordPress Playground instance.
WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Test this pull request with WordPress Playground.

Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit.

Copy link
Contributor

github-actions bot commented Dec 2, 2024

Size Change: 0 B

Total Size: 2.38 MB

compressed-size-action

@Aljullu Aljullu changed the base branch from trunk to release/9.4 December 2, 2024 16:53
@github-actions github-actions bot added focus: e2e tests Issues related to e2e tests focus: monorepo infrastructure Issues and PRs related to monorepo tooling. labels Dec 2, 2024
@github-actions github-actions bot removed focus: e2e tests Issues related to e2e tests focus: monorepo infrastructure Issues and PRs related to monorepo tooling. labels Dec 2, 2024
@Aljullu Aljullu marked this pull request as ready for review December 2, 2024 17:04
@Aljullu Aljullu requested review from gigitux and mdperez86 December 2, 2024 17:04
@woocommercebot woocommercebot requested a review from a team December 2, 2024 17:04
Copy link
Contributor

github-actions bot commented Dec 2, 2024

Hi @gigitux, @mdperez86, @woocommerce/woo-fse

Apart from reviewing the code changes, please make sure to review the testing instructions and verify that relevant tests (E2E, Unit, Integration, etc.) have been added or updated as needed.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

Copy link
Contributor

@gigitux gigitux left a comment

Choose a reason for hiding this comment

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

Screen.Capture.on.2024-12-02.at.18-21-13.mp4

LGTM! I left a minor comment, but it should be straightforward to fix!

This PR is targeting release/9.4 because I think we should do a Point Release Request. In theory, the breaking changes won't be released until Gutenberg 19.9 (scheduled for December 18th), so we could target WC 9.5 (scheduled for December 16th). But I think it's risky because if the WC release is delayed just a couple of days, there would be an incompatibility between Gutenberg and WooCommerce. So I think it's safer to target WC 9.4.

Great catch! I agree with you!

It is not related to this PR, but I noticed this warning:

image

@Aljullu
Copy link
Contributor Author

Aljullu commented Dec 2, 2024

Thanks for the review, @gigitux!

It is not related to this PR, but I noticed this warning:

Hmm, do you know how I can reproduce this? I tried with Nightly enabled and disabled and going from the dashboard or hard reloding and I'm not able to reproduce in any case. 😕

@Aljullu Aljullu merged commit bdef42a into release/9.4 Dec 2, 2024
38 checks passed
@Aljullu Aljullu deleted the fix/cys-router branch December 2, 2024 18:19
@gigitux
Copy link
Contributor

gigitux commented Dec 3, 2024

Hmm, do you know how I can reproduce this? I tried with Nightly enabled and disabled and going from the dashboard or hard reloding and I'm not able to reproduce in any case. 😕

I'm able to replicate opening the Assembler. Given that it is a warning, it is possible that it shows up only when WP_DEBUG is true.

Aljullu added a commit that referenced this pull request Dec 3, 2024
@Aljullu Aljullu restored the fix/cys-router branch December 4, 2024 08:22
@Aljullu Aljullu deleted the fix/cys-router branch December 4, 2024 08:22
@Aljullu
Copy link
Contributor Author

Aljullu commented Dec 4, 2024

I'm able to replicate opening the Assembler. Given that it is a warning, it is possible that it shows up only when WP_DEBUG is true.

Oh right, I think SCRIPT_DEBUG was also required and that's why I couldn't see the error. I could finally reproduce an opened an issue here: #53435. Thanks, @gigitux!

@naman03malhotra naman03malhotra modified the milestones: 9.5.0, 9.4.0 Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
focus: customize-your-store Issues related to the Customize Your Store onboarding flow. plugin: woocommerce Issues related to the WooCommerce Core plugin.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants