Interactivity API: implement the new store() API
#11071
Conversation
We can change this to use deepsignal once this PR is merg...We can change this to use deepsignal once this PR is merged. github.com/luisherranz/deepsignal/pull/38
woocommerce-blocks/assets/js/interactivity/slots.js Lines 29 to 40 in d196254 🚀 This comment was generated by the automations bot based on a
|
|
The release ZIP for this PR is accessible via: Script Dependencies ReportThere is no changed script dependency between this branch and trunk. This comment was automatically generated by the TypeScript Errors Report
assets/js/blocks/collection-filters/inner-blocks/attribute-filter/frontend.ts
assets/js/blocks/collection-filters/inner-blocks/price-filter/frontend.ts assets/js/blocks/collection-filters/inner-blocks/stock-filter/frontend.ts packages/interactivity-components/dropdown/index.ts |
|
Size Change: +1.62 kB (0%) Total Size: 1.61 MB
ℹ️ View Unchanged
|
|
I've extracted the getters so different instances of them run "in the scope of each component". It's a bit like hooks, but automatic 😄 I've also refactored the I'll keep thinking if |
|
|
||
| const actionHandlers = { | ||
| get: ( target, key, receiver ) => { | ||
| const result = Reflect.get( target, key, receiver ); |
There was a problem hiding this comment.
const result = Reflect.get( target, key, receiver );
Note: This could also be a getter if for some reason this runs outside of a scope. I can't think of any case where a getter might run outside of a scope and still need access to the default namespace, but maybe we find it in the future.
So there's nothing to do, this is just a note in case we find that case in the future.
assets/js/interactivity/store.ts
Outdated
| if ( getter ) { | ||
| const scope = getScope(); | ||
| if ( scope ) { | ||
| scope.getters = scope.getters || new Map(); |
There was a problem hiding this comment.
Maybe we should move this to an external Map as well: scope -> getters, to avoid adding non-user-facing stuff to the scope.
There was a problem hiding this comment.
Currently, the scope is not publicly accessible. Developers use getElement and getContext, which return only a subset of the current scope. But we can move getters outside and use a WeakMap to link them to the scope related. I think it makes sense, anyway, so I'll make this change. 👍
assets/js/interactivity/store.ts
Outdated
| } ) | ||
| ); | ||
| } | ||
| return scope.getters.get( getter ).value; |
There was a problem hiding this comment.
Note: We are going to need a way to access the computed when we optimize wp-text and wp-bind to pass signals down. It doesn't have to be a nice API, so maybe something as simple as getComputed(obj, "prop");
| } | ||
|
|
||
| // Check if the property is an object. | ||
| if ( isObject( result ) ) return proxify( result, ns ); |
There was a problem hiding this comment.
Note: This is reusing the isObject from deepMerge which excludes arrays, although arrays could potentially contain functions/generators. For now, I don't think we need to support that as I can't think of any case where someone would like to have actions inside an array.
So there's nothing to do, this is just a note in case we find that case in the future.
There was a problem hiding this comment.
Yup, that's the reason I opted to use isObject. If we find we need to support actions inside arrays, we can change it in the future. 🙂
|
Another thing I've found is that we are using forward slashes in namespaces, e.g. But we didn't allow that character in namespaces for directives defining it explicitly, so the following didn't work: <div data-wp-watch="woocommerce/product-button::callbacks.start animation"></div>Fixed in 5d91a56. |
There was a problem hiding this comment.
I have reviewed the code and tested this PR via #11558 and couldn't spot anything off. However, as I mentioned in #11558 (review), I still feel I'm missing a lot of context around how things work with the Interactivity API, so I don't think I'm entitled to approve or reject this PR. I will let the others take a look.
Nevertheless, I left a couple of comments in the code. They are both minor suggestions/questions, so feel free to disregard them.
| useEffect( () => { | ||
| // Prefetch the page if it is in the directive options. | ||
| if ( link?.prefetch ) { | ||
| prefetch( href ); | ||
| // prefetch( href ); |
There was a problem hiding this comment.
Why was this commented out? Is it intended to leave it like this?
There was a problem hiding this comment.
Uh, funny. 😄 That's actually the code we have in Gutenberg right now:
I'd leave it as it is for now, although that should change eventually.
| const isObject = ( item: unknown ): boolean => | ||
| !! item && typeof item === 'object' && ! Array.isArray( item ); |
There was a problem hiding this comment.
I don't know if you can have @woocommerce dependencies in this file. But in case you can, we have an isObject function in @woocommerce/types which might be useful.
There was a problem hiding this comment.
I would prefer not to depend on other files not part of the Interactivity API runtime. 🤔
…`store()` API (#11558) * Refactor Product Button with new store() API * Use `wc_initial_state` in Product Button * Fix namespace * Remove unnecessary state * Test namespaces in directive paths * Add test context with namespace * Simplify woo-test context * Move addToCart and animations to a file * Do not pass `rawStore` to `afterLoad` callbacks * Move callbacks and actions back to the main file Because the animation was broken. * Remove selectors in favor of state * Use default ns in `getContext` for state and actions * Remove `afterLoad` callback * Remove unnecessary ns * Fix getContext in add-to-cart * Replace namespace and delete unnecessary store * Pass context types only once * Use an alternative for requestIdleCallback * Add previous react code for notices * Add namespace to Product Collection block * Replace getTextButton with getButtonText * Add block name to the ProductCollection namespace * fix style HTML code * Remove circular deps error on the Interactivity API * Product Gallery block: Migrate to new Interactivity API store (#11721) * Migrate Product Gallery block to new Interactivity API store * Fix some references * Add missing data-wc-interactive * Fix an additional namespace * Remove unnecessary click handler * Dialog working * Refactor action names * Reindex PHP array There was some missing indexes, which turned the array into an object in JS. * Remove unused event handlers * Move next/previous logic to external function * Move StorePart util to the types folder * Rename namespace to `woocommerce/product-gallery` * Undo product collection namespace renaming * Remove unnecessary namespace * Don't hide the large image on page load * Minor refactorings * Fix eslint error * Fix php cs errors with spacing and double arrows alignment * Disable no-use-before-define rule for eslint * Disable @typescript-eslint/ban-types rule for eslint * Fix parsed context error in e2e tests * Fix context parser for Thumbnail image * Move store to the top of the frontend file * Add interactivity api utils to the @woocommerce/utils alias * Replace deprecated event attribute --------- Co-authored-by: Luis Herranz <luisherranz@gmail.com> Co-authored-by: David Arenas <david.arenas@automattic.com> Co-authored-by: roykho <roykho77@gmail.com> --------- Co-authored-by: David Arenas <david.arenas@automattic.com> Co-authored-by: Luigi Teschio <gigitux@gmail.com> Co-authored-by: Alexandre Lara <allexandrelara@gmail.com> Co-authored-by: roykho <roykho77@gmail.com>
Replace with an interactive block that calls `actions.sel...Replace with an interactive block that calls `actions.selectImage`.
woocommerce-blocks/assets/js/blocks/product-gallery/frontend.tsx Lines 86 to 97 in 6413756 🚀 This comment was generated by the automations bot based on a
|
Replace with an interactive block that calls `actions.sel...Replace with an interactive block that calls `actions.selectImage`.
woocommerce-blocks/assets/js/blocks/product-gallery/frontend.tsx Lines 86 to 97 in b67b264 🚀 This comment was generated by the automations bot based on a
|
Replace with an interactive block that calls `actions.sel...Replace with an interactive block that calls `actions.selectImage`.
woocommerce-blocks/assets/js/blocks/product-gallery/frontend.tsx Lines 90 to 101 in f665a38 🚀 This comment was generated by the automations bot based on a
|
There was a problem hiding this comment.
@gigitux can you apply the changes that you did in #11858 also here please?
Are the e2e tests already fixed in trunk? Who's taking care of that? I'd like to merge this PR to continue the migration.
As there are conflicts with #11858, I'm going to anticipately approve this PR, but the fix in that PR needs to be applied here as well before merging.
…o interactivity-api-new-store-api
Replace with an interactive block that calls `actions.sel...Replace with an interactive block that calls `actions.selectImage`.
woocommerce-blocks/assets/js/blocks/product-gallery/frontend.tsx Lines 90 to 101 in 09821c9 🚀 This comment was generated by the automations bot based on a
|
…ivity-api-new-store-api
Replace with an interactive block that calls `actions.sel...Replace with an interactive block that calls `actions.selectImage`.
woocommerce-blocks/assets/js/blocks/product-gallery/frontend.tsx Lines 90 to 101 in fa5932c 🚀 This comment was generated by the automations bot based on a
|
luisherranz
force-pushed
the
interactivity-api-new-store-api
branch
from
fa5932c
to
09821c9
Compare
Replace with an interactive block that calls `actions.sel...Replace with an interactive block that calls `actions.selectImage`.
woocommerce-blocks/assets/js/blocks/product-gallery/frontend.tsx Lines 90 to 101 in 09821c9 🚀 This comment was generated by the automations bot based on a
|
Replace with an interactive block that calls `actions.sel...Replace with an interactive block that calls `actions.selectImage`.
woocommerce-blocks/assets/js/blocks/product-gallery/frontend.tsx Lines 90 to 101 in 8f675cd 🚀 This comment was generated by the automations bot based on a
|
What
This PR implements the new
store()API as specified in the proposal: WordPress/gutenberg#53586It also updates the current Interactivity API implementation to match the latest features and improvements developed in WordPress/gutenberg.
Will close #11065
Why
To test the new API with actual use cases so we can fine-tune and shape it.
Testing Instructions
Please, go to the following PRs that are built on top of this branch to test these changes:
store()API #11558WooCommerce Visibility
Required: