Size Change: -4 B (0%)

Total Size: 1.39 MB

_{compressed-size-action}

The PR is scoped down to only affect getAllBlocks. My first thought was to add this to wpDataSelect, which would affect many more tests. I ran the test suite and modifying wpDataSelect is also making the tests pass. It's a more future-proof solution.

I wonder what others think, should we add this to wpDataSelect or keep it scoped to getAllBlocks?

My first thought was to add this to wpDataSelect, which would affect many more tests. I ran the test suite and modifying wpDataSelect is also making the tests pass. It's a more future-proof solution.

I'd agree with your intuition here. I only see wpDataSelect being used in 6 files, which seems like a relatively small surface area to manage.

1. packages/e2e-test-utils/src/are-pre-publish-checks-enabled.js
2. packages/e2e-test-utils/src/get-all-blocks.js
3. packages/e2e-test-utils/src/get-current-post-content.js
4. packages/e2e-test-utils/src/get-edited-post-content.js
5. packages/e2e-test-utils/src/index.js
6. packages/e2e-tests/experimental-features.js

I can also imagine others running into the same problem in the future, so I'd vote for making the change to wpDataSelect itself. Worse comes to worse, we'd break e2e testing for a short period of time, at which point we'd revert these relatively compact, non-user facing changes.

NB: If we end up merging changes scoped only to getAllBlocks, it'd be great to have a follow-up ticket to update other files that reference wpDataSelect

I wonder what others think, should we add this to wpDataSelect or keep it scoped to getAllBlocks?

I think we can restrict this to getAllBlocks for now that has problems with block deprecations. Running parse and stringify for every wp.data.select call might not be needed and definitely is not needed in GB repo now.

We should not forget that both are public APIs and are used outside of GB as well. Since there are no created issues for either of them, IMO it's okay to land the only needed change for getAllBlocks.

We should not forget that both are public APIs and are used outside of GB as well. Since there are no created issues for either of them, IMO it's okay to land the only needed change for getAllBlocks.

That makes sense 👍 .

In that case, could we leave more explicit documentation somewhere? It seems easy to overlook the fact that wpDataSelect will only behave as expected if returning a serializable value. It might read something like "If using the wpDataSelect e2e helper, then ensure that the return value from the selector is serializable (e.g. NOT function, Dom element, etc.)."

In that case, could we leave more explicit documentation somewhere? It seems easy to overlook the fact that wpDataSelect will only behave as expected if returning a serializable value. It might read something like "If using the wpDataSelect e2e helper, then ensure that the return value from the selector is serializable (e.g. function, Dom element, etc.)."

Agreed! We can do this in a follow up.

I'll merge this though now, as is needed for #30804

Thanks for all the work here!