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

Fix context injection loop #26998

Merged
merged 4 commits into from
Nov 16, 2020
Merged

Fix context injection loop #26998

merged 4 commits into from
Nov 16, 2020

Conversation

youknowriad
Copy link
Contributor

This refactors the context injection to rely on register_block_type hook instead, that way it's less dependent on the implementation of the block rendering in Core.

This is intended to fix the current e2e failures.

* @param array $parsed_block Block being rendered, filtered by `render_block_data`.
*/
$context = apply_filters( 'render_block_context', $context, $parsed_block );
if ( isset( $wp_query->tax_query->queried_terms['post_tag'] ) ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ntsekouras I'm not sure I understand why we need to inejct the "query" context here. This compact function should be only used to implement the context support behavior and not inject things into the context. unless, these are "default injections" things that should be injected to all blocks?

Copy link
Contributor

Choose a reason for hiding this comment

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

The first injection for categories was from Enrique and when I implemented the tags support followed that lead, without digging deep. After some more recent explorations with context (without being sure yet), I believe this was implemented by Enrique to make the QueryLoop inherit global context if is not wrapped by Query block.

I believe this used to work for the above case (not wrapped in Query), but many changes to many things (like FSE navigation, context, Query etc) made it not to use these values.

For me this code is a part to the exploration of inheriting global context to Query block, but it's not clear to me how to handle this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ntsekouras Would you be able to check that everything works properly for the query block in this PR?

Copy link
Contributor

@ntsekouras ntsekouras Nov 16, 2020

Choose a reason for hiding this comment

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

Query works fine, but I said above, I think this is some leftover code that needs reworking as a part of global context inheritance problem.

-- I tested with my change in the suggestion for the typo qery -> query

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, that's fine, I'm just making sure I don't regress anything and fix the currently failing e2e tests. Thanks for testing.

@github-actions
Copy link

github-actions bot commented Nov 16, 2020

Size Change: 0 B

Total Size: 1.19 MB

ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.77 kB 0 B
build/api-fetch/index.js 3.42 kB 0 B
build/autop/index.js 2.84 kB 0 B
build/blob/index.js 664 B 0 B
build/block-directory/index.js 8.71 kB 0 B
build/block-directory/style-rtl.css 943 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/index.js 133 kB 0 B
build/block-editor/style-rtl.css 11.3 kB 0 B
build/block-editor/style.css 11.3 kB 0 B
build/block-library/editor-rtl.css 8.9 kB 0 B
build/block-library/editor.css 8.9 kB 0 B
build/block-library/index.js 147 kB 0 B
build/block-library/style-rtl.css 8.1 kB 0 B
build/block-library/style.css 8.1 kB 0 B
build/block-library/theme-rtl.css 792 B 0 B
build/block-library/theme.css 793 B 0 B
build/block-serialization-default-parser/index.js 1.87 kB 0 B
build/block-serialization-spec-parser/index.js 3.06 kB 0 B
build/blocks/index.js 48 kB 0 B
build/components/index.js 171 kB 0 B
build/components/style-rtl.css 15.3 kB 0 B
build/components/style.css 15.3 kB 0 B
build/compose/index.js 9.9 kB 0 B
build/core-data/index.js 14.8 kB 0 B
build/data-controls/index.js 821 B 0 B
build/data/index.js 8.74 kB 0 B
build/date/index.js 11.2 kB 0 B
build/deprecated/index.js 768 B 0 B
build/dom-ready/index.js 571 B 0 B
build/dom/index.js 4.92 kB 0 B
build/edit-navigation/index.js 11.1 kB 0 B
build/edit-navigation/style-rtl.css 881 B 0 B
build/edit-navigation/style.css 885 B 0 B
build/edit-post/index.js 306 kB 0 B
build/edit-post/style-rtl.css 6.51 kB 0 B
build/edit-post/style.css 6.49 kB 0 B
build/edit-site/index.js 23.2 kB 0 B
build/edit-site/style-rtl.css 3.98 kB 0 B
build/edit-site/style.css 3.98 kB 0 B
build/edit-widgets/index.js 26.3 kB 0 B
build/edit-widgets/style-rtl.css 3.16 kB 0 B
build/edit-widgets/style.css 3.16 kB 0 B
build/editor/editor-styles-rtl.css 476 B 0 B
build/editor/editor-styles.css 478 B 0 B
build/editor/index.js 42.5 kB 0 B
build/editor/style-rtl.css 3.85 kB 0 B
build/editor/style.css 3.85 kB 0 B
build/element/index.js 4.62 kB 0 B
build/escape-html/index.js 735 B 0 B
build/format-library/index.js 6.86 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 2.16 kB 0 B
build/html-entities/index.js 623 B 0 B
build/i18n/index.js 3.57 kB 0 B
build/is-shallow-equal/index.js 698 B 0 B
build/keyboard-shortcuts/index.js 2.52 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.1 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.32 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.43 kB 0 B
build/priority-queue/index.js 790 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/reusable-blocks/index.js 3.05 kB 0 B
build/rich-text/index.js 13.3 kB 0 B
build/server-side-render/index.js 2.77 kB 0 B
build/shortcode/index.js 1.69 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 4.05 kB 0 B
build/viewport/index.js 1.84 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.22 kB 0 B

compressed-size-action

@youknowriad
Copy link
Contributor Author

It seems my fix here is not good enough, I think we regressed on block context.

lib/compat.php Outdated Show resolved Hide resolved
Co-authored-by: Nik Tsekouras <ntsekouras@outlook.com>
@youknowriad
Copy link
Contributor Author

So this seems to fix most of the end2end tests. There are some remaining failures which I believe were introduced by #26903

@youknowriad youknowriad merged commit a89276d into master Nov 16, 2020
@youknowriad youknowriad deleted the try/fix-context-compat branch November 16, 2020 14:07
@github-actions github-actions bot added this to the Gutenberg 9.4 milestone Nov 16, 2020
@youknowriad youknowriad added the [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. label Nov 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants