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.

Sign up for GitHub

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

Jump to bottom

Blaze: only enqueue script in post editor #28187

Merged
merged 1 commit into from
Jan 5, 2023
Merged

Blaze: only enqueue script in post editor #28187

merged 1 commit into from
Jan 5, 2023

Conversation

jeherve
Copy link
Member

@jeherve jeherve commented Jan 5, 2023

Fixes #28184

Changes proposed in this Pull Request:

  1. We do not need Blaze in the site editor, since we cannot promote specific posts from there.
  2. The Blaze panel relies on @wordpress/editor, which we cannot use in the site editor or the widget editor.
  3. When we enqueue the Blaze panel in the site editor or the widget editor, we run into this issue: Editor: solve conflict between Jetpack blocks' @wordpress/editor dependencies and Block-based Widgets Editor #20357. This causes Fatal errors just like the one we experienced and worked around in Revert and fix "Revert "Assets: do not use the new method yet (#21760)"" #21763.

To solve this issue, we switch to using the admin_enqueue_scripts hook to decide whether when to enqueue our script. It allows us to conditionally enqueue the file based on the page we're on, which is something the enqueue_block_editor_assets hook does not allow yet.

Other information:

  • Have you written new tests for your changes, if applicable?
  • Have you checked the E2E test CI results, and verified that your changes do not break them?
  • Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

  • p1672870282744879-slack-C01U2KGS2PQ

Does this pull request change what data or activity we track or use?

  • No

Testing instructions:

  • Start on a WordPress.com simple site or a WoA site using a block theme like Twenty Twenty Three.
  • Go to Posts > Add New and publish a new post; you should see the Blaze panel after publishing
  • Go to Appearance > Site editor, the page should load just fine.

@jeherve
Blaze: only enqueue script in post editor
fd29b2e 
Fixes #28184

1. We do not need Blaze in the site editor, since we cannot promote specific posts from there.
2. The Blaze panel relies on @wordpress/editor, which we cannot use in the site editor or the widget editor.
3. When we enqueue the Blaze panel in the site editor or the widget editor, we run into this issue: #20357. This causes Fatal errors just like the one we experienced and worked around in #21763.

To solve this issue, we switch to using the admin_enqueue_scripts hook to decide whether when to enqueue our script. It allows us to conditionally enqueue the file based on the page we're on, which is something the enqueue_block_editor_assets hook does not allow yet.
@jeherve jeherve added [Type] Bug When a feature is broken and / or not performing as intended [Status] Needs Review To request a review from Crew. Label will be renamed soon. [Pri] High [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack labels Jan 5, 2023
@jeherve jeherve self-assigned this Jan 5, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jan 5, 2023 •
edited
Loading

Are you an Automattician? You can now test your Pull Request on WordPress.com. On your sandbox, run bin/jetpack-downloader test jetpack fix/fatal-site-editor-blaze to get started. More details: p9dueE-5Nn-p2

@github-actions
Copy link
Contributor

github-actions bot commented Jan 5, 2023 •
edited
Loading

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

  • ✅ Include a description of your PR changes.
  • ✅ All commits were linted before commit.
  • ✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
  • ✅ Add testing instructions.
  • ✅ Specify whether this PR includes any changes to data or privacy.
  • ✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.

coder-karen
coder-karen approved these changes Jan 5, 2023
View reviewed changes
Copy link
Contributor

@coder-karen coder-karen left a comment

Choose a reason for hiding this comment

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

I can confirm this fixes the site editor issue.

@coder-karen coder-karen added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Jan 5, 2023
@coder-karen coder-karen merged commit a2f421c into trunk Jan 5, 2023
@coder-karen coder-karen deleted the fix/fatal-site-editor-blaze branch January 5, 2023 08:50
@github-actions github-actions bot removed the [Status] Ready to Merge Go ahead, you can push that green button! label Jan 5, 2023
coder-karen pushed a commit that referenced this pull request Jan 5, 2023
@jeherve @coder-karen
[not verified] Blaze: only enqueue script in post editor (#28187)
ecd8f17
@coder-karen
Copy link
Contributor

Cherry-picked to the release branch in ecd8f17

@coder-karen coder-karen mentioned this pull request Jan 5, 2023
Closed
3 tasks
@kraftbj
Copy link
Contributor

kraftbj commented Jan 5, 2023

Looking for upstream assistance in ensuring this type of bug is harder to occur. WordPress/gutenberg#46900 and https://core.trac.wordpress.org/ticket/57421 (props @jeherve)

n3f pushed a commit that referenced this pull request Jan 11, 2023
@jeherve @n3f
[not verified] Blaze: only enqueue script in post editor (#28187)
89cec82
@jeherve jeherve mentioned this pull request Feb 10, 2023
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@coder-karen coder-karen coder-karen approved these changes

@dereksmart dereksmart Awaiting requested review from dereksmart

Assignees

@jeherve jeherve

Labels
[Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack [Package] Blaze [Pri] High [Status] In Progress [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Blaze panel conflicts with Site Editor
3 participants
@jeherve @coder-karen @kraftbj