This repository has been archived by the owner on Feb 23, 2024. It is now read-only.
Interactivity API: add afterLoad callbacks to store() function
#10338
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The release ZIP for this PR is accessible via: Script Dependencies ReportThe
This comment was automatically generated by the TypeScript Errors Report
assets/js/atomic/blocks/product-elements/button/frontend.tsx
assets/js/base/context/providers/cart-checkout/checkout-events/index.tsx assets/js/base/utils/create-notice.ts assets/js/blocks/cart-checkout-shared/sidebar-notices/index.tsx assets/js/blocks/classic-template/test/utils.ts assets/js/blocks/product-collection/inspector-controls/author-control.tsx assets/js/blocks/product-collection/inspector-controls/taxonomy-controls/index.tsx assets/js/blocks/product-collection/inspector-controls/taxonomy-controls/taxonomy-item.tsx assets/js/blocks/product-gallery/edit.tsx assets/js/blocks/product-template/edit.tsx assets/js/editor-components/default-notice/index.tsx assets/js/interactivity/router.js assets/js/interactivity/store.js assets/js/utils/notices.ts node_modules/@jest/test-result/build/index.d.ts node_modules/@types/wordpress__editor/components/autocompleters.d.ts node_modules/@types/wordpress__editor/components/post-taxonomies/index.d.ts node_modules/@types/wordpress__editor/store/actions.d.ts node_modules/@types/wordpress__editor/store/selectors.d.ts node_modules/@wordpress/core-data/build-types/index.d.ts node_modules/preact/src/jsx.d.ts packages/checkout/components/store-notices-container/test/index.tsx |
DAreRodz
commented
Jul 24, 2023
assets/js/interactivity/index.js
Outdated
|
|
||
| /** | ||
| * Initialize the Interactivity API. | ||
| */ | ||
| document.addEventListener( 'DOMContentLoaded', async () => { | ||
| registerDirectives(); | ||
| await init(); | ||
| runStoreCallbacks( 'afterLoad' ); |
There was a problem hiding this comment.
I'm unsure if it is better to call runStoreCallbacks before or after init. I opted for the second option, but we can change it if you feel appropriate.
cc: @luisherranz
There was a problem hiding this comment.
It's correct. Before init is reserved for beforeLoad:
await runStoreCallbacks( 'beforeLoad' );
await init();
runStoreCallbacks( 'afterLoad' );There was a problem hiding this comment.
Maybe we should rename init to load 😆
|
Size Change: +787 B (0%) Total Size: 1.34 MB
ℹ️ View Unchanged
|
DAreRodz
commented
Jul 24, 2023
assets/js/interactivity/store.js
Outdated
Comment on lines
49
to
50
| export const runStoreCallbacks = ( key ) => { | ||
| storeCallbacks[ key ]?.forEach( ( cb ) => cb( rawStore ) ); |
There was a problem hiding this comment.
Here I've been considering how this function should work during client-side navigation, taking in mind that, in the future, some blocks will be loaded when navigating to new pages. To prevent calling the same callbacks again, maybe we can remove all registered callbacks once executed, and call runStoreCallbacks() again after each new render.
If this is not suitable for certain callback types (e.g., afterNavigation? 🤷 ), we could add a parameter to indicate that the executed callbacks should be removed afterward.
There was a problem hiding this comment.
Good point. Maybe we can use a Map to avoid adding the same reference twice, and mark the executed callbacks to avoid executing them again.
const callbacks = {
afterLoad: new Map(),
};
export const store = ({ state, ...block } = {}, { afterLoad } = {}) => {
deepMerge(rawStore, block);
deepMerge(rawState, state);
if (afterLoad && !callbacks.afterLoad.has(afterLoad))
callbacks.afterLoad.set(afterLoad, false);
};
export const runStoreCallbacks = (key) => {
callbacks[key]?.entries(([cb, executed]) => {
if (!executed) {
cb(rawStore);
callbacks[key].set(cb, true);
}
});
};But let's wait to do so until we are actually testing this with full-page client-side navigation.
7 tasks
gigitux
force-pushed
the
add/6949-interactivity-api-product-button
branch
from
54cb0ff to
efbfe3b
Compare
luisherranz
suggested changes
Jul 25, 2023
assets/js/interactivity/store.js
Outdated
Comment on lines
35
to
50
| const storeCallbacks = {}; | ||
| const rawState = getSerializedState(); | ||
| export const rawStore = { state: deepSignal( rawState ) }; | ||
|
|
||
| if ( typeof window !== 'undefined' ) window.store = rawStore; | ||
|
|
||
| export const store = ( { state, ...block } ) => { | ||
| export const store = ( { state, ...block }, callbacks = {} ) => { | ||
| deepMerge( rawStore, block ); | ||
| deepMerge( rawState, state ); | ||
| Object.entries( callbacks ).forEach( ( [ key, cb ] ) => { | ||
| ( storeCallbacks[ key ] = storeCallbacks[ key ] || [] ).push( cb ); | ||
| } ); | ||
| }; | ||
|
|
||
| export const runStoreCallbacks = ( key ) => { | ||
| storeCallbacks[ key ]?.forEach( ( cb ) => cb( rawStore ) ); |
There was a problem hiding this comment.
I don't expect to have an unlimited number of callbacks so I think we can hardcode their names. I also don't intend to use the second parameter only for callbacks, but for options in general.
Also, I'm not 100% sure about it yet, but maybe instead of an array, we could use a Set to avoid adding the same callback more than once?
Something like this:
const callbacks = {
afterLoad: new Set(),
};
export const store = ({ state, ...block } = {}, options = {}) => {
deepMerge(rawStore, block);
deepMerge(rawState, state);
if (options.afterLoad) callbacks.afterLoad.add(options.afterLoad);
};
export const runStoreCallbacks = (key) => {
callbacks[key]?.forEach((cb) => cb(rawStore));
};
assets/js/interactivity/store.js
Outdated
Comment on lines
49
to
50
| export const runStoreCallbacks = ( key ) => { | ||
| storeCallbacks[ key ]?.forEach( ( cb ) => cb( rawStore ) ); |
There was a problem hiding this comment.
Good point. Maybe we can use a Map to avoid adding the same reference twice, and mark the executed callbacks to avoid executing them again.
const callbacks = {
afterLoad: new Map(),
};
export const store = ({ state, ...block } = {}, { afterLoad } = {}) => {
deepMerge(rawStore, block);
deepMerge(rawState, state);
if (afterLoad && !callbacks.afterLoad.has(afterLoad))
callbacks.afterLoad.set(afterLoad, false);
};
export const runStoreCallbacks = (key) => {
callbacks[key]?.entries(([cb, executed]) => {
if (!executed) {
cb(rawStore);
callbacks[key].set(cb, true);
}
});
};But let's wait to do so until we are actually testing this with full-page client-side navigation.
luisherranz
approved these changes
Jul 25, 2023
|
This PR has been marked as If deemed still relevant, the pr can be kept active by ensuring it's up to date with the main branch and removing the stale label. |
github-actions
bot
added
the
status: stale
gigitux
removed
the
status: stale
* implement animation * improve logic * refactor logic * refactor code * address feedback about code style * add support for woocommerce_add_to_cart_quantity * Fix animation flickering * Introduce wp-effect, reduce the amount of numberOfItem variables to 2 and consolidate animation status * add support for added class * Remove unnecessary selector * Don't fetch cart if it was already fetched * remove added class --------- Co-authored-by: Luis Herranz <luisherranz@gmail.com>
|
I think we can also merge this one and keep working on the original PR. Don't you think? |
…ithub.com/woocommerce/woocommerce-blocks into interactivity-api-after-load-callback
6 tasks
gigitux
added a commit
that referenced
this pull request
Aug 10, 2023
* Update Interactivity API JS files * Disable TS checks in the Interactivity API for now * Add new SSR files * Replace wp_ prefixes with wc_ ones * Replace wp- prefix with wc- * Replace guternberg_ prefix with woocommerce_ * Remove file comments from Gutenberg * Rename files with `wp` prefix * Fix code to load Interactivity API php files * Remove TODO comments * Replace @WordPress with @woocommerce * Update Webpack configuration * Fix directive prefix * Remove interactivity folder from tsconfig exclude * Add client-side navigation meta tag code * Remove unneeded blocks.php file * Fix store tag id * Register Interactivity API runtime script * Fix Interactivity API runtime registering * Remove all files related to directive processing in PHP * Move json_encode to Store's render method * WIP * WIP * WIP * WIP * Preserve previous context * Ignore Minicart block on client-side navigation * Refresh page on store updatRefresh page on store updatee * Refactor logic * Add console error when a path is missing * fix PHP lint error * WIP store * use store approach * update jest configuration * restore Mini Cart changes * move cart store subscription to interactivity package * move interactivity flag * format HTML * move addToCartText to the context * Load product-query stylesheet when rendering the Products block * update sideEffects array * fix catch * rename moreThanOneItem to isThereMoreThanOneItem * improve how scripts are enqueued * update default value for the filter woocommerce_blocks_enable_interactivity_api * Update assets/js/atomic/blocks/product-elements/button/block.json Co-authored-by: Albert Juhé Lluveras <contact@albertjuhe.com> * Update assets/js/interactivity/cart/cart-store.ts Co-authored-by: Albert Juhé Lluveras <contact@albertjuhe.com> * fix block.json * remove updateStore function * restore interactivity api changes * import cart store * show notice when there is an error * add logic to dequeue script on classic themes and block themes * imrpove logic about notice * Interactivity API: add `afterLoad` callbacks to `store()` function (#10338) * show notice when there is an error * Add initial implementation for store callbacks * Run `afterLoad` callbacks after `init` * Move cart state subscription to Product button * Remove cart-store from Interactivity API internals * Change callbacks with options and save only afterLoad callbacks * ProductButton: Add animation (#10351) * implement animation * improve logic * refactor logic * refactor code * address feedback about code style * add support for woocommerce_add_to_cart_quantity * Fix animation flickering * Introduce wp-effect, reduce the amount of numberOfItem variables to 2 and consolidate animation status * add support for added class * Remove unnecessary selector * Don't fetch cart if it was already fetched * remove added class --------- Co-authored-by: Luis Herranz <luisherranz@gmail.com> --------- Co-authored-by: Luigi <gigitux@gmail.com> Co-authored-by: Luis Herranz <luisherranz@gmail.com> * update deepsignal * remove added class * update deepsignal * Interactivity API and Product Button: Add E2E tests (#10036) * Add FrontendUtils class * fix conflicts * use locator * restore click usage * Product Button: Add E2E test * fix util * fix E2E tests * remove comment * Add E2E test to ensure that woocommerce_product_add_to_cart_text works * update sideEffects array * add zip and unzip as package * fix wp-env configuration * fix E2E test * add report * try now * try now * try now * fix E2E test * E2E: Add documentation for testing actions and filters. Fixes #10135 (#10206) * update description * fix label * rename files * make requestUtils private * remove page.goto * use toHaveCount * use productsToDisplay variable * fix E2E tests * rename class utils --------- Co-authored-by: Daniel Dudzic <daniel.dudzic@automattic.com> --------- Co-authored-by: David Arenas <david.arenas@automattic.com> Co-authored-by: Luis Herranz <luisherranz@gmail.com> Co-authored-by: Albert Juhé Lluveras <contact@albertjuhe.com> Co-authored-by: Daniel Dudzic <daniel.dudzic@automattic.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implements a feature that will allow developers to pass callbacks to the Interactivity API
storefunction, e.g.,Discussed in #10006 (comment)
Testing
Follow testing steps defined in #10006.
WooCommerce Visibility