-
Notifications
You must be signed in to change notification settings - Fork 10.7k
[WIP] Add to Cart with Options - Refactor product context and quantity selection to avoid imperative DOM manipulation #61730
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
base: trunk
Are you sure you want to change the base?
Conversation
|
Claude finished @samueljseay's task —— View job Backwards Compatibility ReviewSummary: MEDIUM Risk - Breaking changes to private/locked JavaScript store APIThis refactoring introduces breaking changes to JavaScript Interactivity API stores that are currently marked as private/locked ( Rationale: The PR renames/restructures internal WordPress Interactivity API stores ( Breaking Changes Detected1. JavaScript Store Renamed -
|
|
|
||
| const { state: productDataState } = store< ProductDataStore >( | ||
| 'woocommerce/product-data', | ||
| const { state: productContextState } = store< ProductDataStore >( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BC Risk: Store name changed from woocommerce/product-data to woocommerce/product-context. Extensions accessing the old locked store will break.
Old: store('woocommerce/product-data')
New: store('woocommerce/product-context')
The state structure also changed:
productDataState.productId→productContextState.currentProductIdproductDataState.variationIdremoved (moved to this store asstate.selectedVariationId)
Consider adding a deprecation shim for the old store name to ease migration.
| selectedAttributes: SelectedAttributes[]; | ||
| productData: NormalizedProductData | NormalizedVariationData | null; | ||
| childProductData: NormalizedProductData | NormalizedVariationData | null; | ||
| activeProduct: NormalizedProductData | NormalizedVariationData | null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BC Risk: setQuantity() signature changed - added third parameter refToDispatchChangeEvent.
Old signature: setQuantity(productId: number, value: number)
New signature: setQuantity(productId: number, value: number, refToDispatchChangeEvent: HTMLInputElement | null)
The parameter is optional at runtime, so existing calls will work but may not dispatch change events. TypeScript consumers will get type errors.
Impact: Extensions calling this locked action will need to update their TypeScript types or pass null as the third parameter.
| if ( $product ) { | ||
| wp_interactivity_state( | ||
| 'woocommerce/product-data', | ||
| 'woocommerce/product-context', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BC Risk: Server-side interactivity state structure changed.
Changes:
- Store name:
woocommerce/product-data→woocommerce/product-context - State key:
productId→currentProductId - Removed:
variationIdproperty
Impact: JavaScript code expecting the old state structure from server-rendered data will break. Extensions filtering this state before output will need updates.
Example of code that will break:
const { productId, variationId } = wp.interactivity.getContext('woocommerce/product-data');Should now be:
const { currentProductId } = wp.interactivity.getContext('woocommerce/product-context');
const { selectedVariationId } = wp.interactivity.getContext('woocommerce/add-to-cart-with-options');| $context_attribute = wp_interactivity_data_wp_context( | ||
| array( | ||
| 'productId' => $child_product_id || ! $product instanceof \WC_Product ? | ||
| 'childProductId' => $child_product_id || ! $product instanceof \WC_Product ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BC Risk: Context attribute key renamed from productId to childProductId.
Impact: JavaScript code reading context.productId from this context will get undefined. Extensions expecting the old key structure will break.
Mitigation: Consider including both keys during a deprecation period:
'childProductId' => $child_product_id || ...,
'productId' => $child_product_id || ..., // @deprecated for BC| $input_attributes['data-wp-bind--step'] = 'woocommerce/add-to-cart-with-options::state.productData.step'; | ||
| $input_attributes['data-wp-watch'] = 'woocommerce/add-to-cart-with-options::callbacks.watchQuantityConstraints'; | ||
| $input_attributes['data-wp-watch---dispatch-change-event'] = 'woocommerce/add-to-cart-with-options::callbacks.maybeDispatchChangeEvent'; | ||
| $input_attributes['data-wp-bind--value'] = 'woocommerce/add-to-cart-with-options::state.inputQuantity'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BC Risk: Data binding attributes changed - namespace moved from woocommerce/product-elements to woocommerce/add-to-cart-with-options.
Changes:
data-wp-bind--min/max/stepnow points towoocommerce/add-to-cart-with-options::state.productData.*instead ofwoocommerce/product-elements::state.productData.*- Added new
data-wp-bind--valuebinding tostate.inputQuantity - Added new watch callback for change event dispatching
Impact: JavaScript code or browser extensions manipulating these DOM attributes will need to account for the new binding sources. Low risk since these are implementation details, but could affect advanced customizations.
|
Claude finished @samueljseay's task —— View job Backwards Compatibility ReviewSummary: MEDIUM Risk - Breaking changes to private/locked JavaScript Interactivity API stores This refactoring introduces breaking changes to JavaScript stores marked as Breaking Changes Detected1. JavaScript Store Renamed and Restructured (HIGH impact for private API users)Files affected:
Changes: // OLD (trunk)
store('woocommerce/product-data', {
state: {
productId: number,
variationId: number | null
},
actions: {
setVariationId: (variationId: number | null) => void
}
})
// NEW (this PR)
store('woocommerce/product-context', {
state: {
currentProductId: number,
childProductId?: number
// variationId moved to woocommerce/add-to-cart-with-options::selectedVariationId
}
// No actions exposed
})Breaking changes:
Code that will break: // Extensions doing this will break:
const { state } = store('woocommerce/product-data', {}, { lock: '...' });
const productId = state.productId; // ❌ undefined
const variationId = state.variationId; // ❌ undefined2. Action Signature Changed (MEDIUM impact for private API users)File: Change: // OLD
setQuantity(productId: number, value: number)
// NEW
setQuantity(productId: number, value: number, refToDispatchChangeEvent: HTMLInputElement | null)The third parameter is optional at runtime ( 3. PHP Interactivity Bindings Updated (LOW impact)Files:
Impact: JavaScript code querying these Non-Breaking Changes (Safe)✅ PHP Hooks: No changes to Testing FocusPrioritize regression testing in these critical areas: 1. Variable Products (CRITICAL)
2. Grouped Products (HIGH)
3. Quantity Changes (HIGH - issue #53031 compatibility)
4. Add to Cart (CRITICAL)
5. Extension Compatibility (HIGH RISK - private API users)Test with popular extensions that might use the locked stores: High Risk Extensions:
Testing approach: Install extension, test variable/grouped product add-to-cart flow, check browser console for errors like: 6. Theme Compatibility (MEDIUM)
Ecosystem Impact AssessmentPayment Gateways: WooCommerce Subscriptions: WooCommerce Bookings/Memberships: Themes: Custom Quantity Selectors: Recommendations
Confidence: 90%Justification: High confidence based on thorough analysis of all 15 changed files and verification against trunk. The breaking changes are clearly identifiable in the diff. Confidence is not 100% because:
Final AssessmentBreaking changes detected: ✅ YES - This PR introduces intentional breaking changes to private/locked JavaScript Interactivity API stores. Is this acceptable?
The refactoring is architecturally sound and achieves its goal of avoiding imperative DOM manipulation. The breaking changes are a necessary consequence of improving the internal architecture. Since the stores are locked with an explicit warning message, the breakage is within acceptable boundaries for private APIs. PHP changes: ✅ Non-breaking - All PHP hooks, public APIs, and templates remain unchanged. Review complete. I've also added inline comments on the specific files/lines where breaking changes occur for easier reference during code review. |
|
|
||
| const { state: productDataState } = store< ProductDataStore >( | ||
| 'woocommerce/product-data', | ||
| const { state: productContextState } = store< ProductDataStore >( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BC Risk (HIGH): Store name changed from woocommerce/product-data to woocommerce/product-context.
Extensions accessing the old locked store will break:
// This will break:
const { state } = store('woocommerce/product-data', {}, { lock: '...' });Mitigation: Consider adding a deprecation shim that proxies the old store name to the new one with console warnings, providing one major version of backwards compatibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to do this. We explicitly made stores private so they can be subject to change right now.
| actions: { | ||
| validateQuantity: ( productId: number, value?: number ) => void; | ||
| setQuantity: ( productId: number, value: number ) => void; | ||
| setQuantity: ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BC Risk (MEDIUM): setQuantity() signature changed - added third parameter.
Old: setQuantity(productId: number, value: number)
New: setQuantity(productId: number, value: number, refToDispatchChangeEvent: HTMLInputElement | null)
The parameter is optional at runtime, so existing calls will function but TypeScript consumers will get type errors. The new parameter enables dispatching synthetic change events for extension compatibility (#53031).
| if ( $product ) { | ||
| wp_interactivity_state( | ||
| 'woocommerce/product-data', | ||
| 'woocommerce/product-context', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BC Risk (MEDIUM): Server-side interactivity state structure changed.
Changes:
- Store name:
woocommerce/product-data→woocommerce/product-context - State key:
productId→currentProductId(line 94) - Removed:
variationIdproperty (moved towoocommerce/add-to-cart-with-options::selectedVariationId)
Impact: JavaScript reading server-rendered state from the old store name or expecting the old keys will break.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again this is all private right now. We must be able to adjust the API as we see fit before releasing these stores to be public.
| $context_attribute = wp_interactivity_data_wp_context( | ||
| array( | ||
| 'productId' => $child_product_id || ! $product instanceof \WC_Product ? | ||
| 'childProductId' => $child_product_id || ! $product instanceof \WC_Product ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BC Risk (LOW): Context attribute key renamed from productId to childProductId.
JavaScript code reading context.productId from this context will receive undefined. Consider including both keys during a deprecation period:
'childProductId' => $child_product_id || ...,
'productId' => $child_product_id || ..., // @deprecated for BC| $input_attributes['data-wp-bind--max'] = 'woocommerce/product-elements::state.productData.max'; | ||
| $input_attributes['data-wp-bind--step'] = 'woocommerce/product-elements::state.productData.step'; | ||
| $input_attributes['data-wp-watch'] = 'woocommerce/add-to-cart-with-options::callbacks.watchQuantityConstraints'; | ||
| $wrapper_attributes['data-wp-bind--hidden'] = 'woocommerce/add-to-cart-with-options-quantity-selector::!state.allowsQuantityChange'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BC Risk (LOW): Data binding namespace changed from woocommerce/product-elements to woocommerce/add-to-cart-with-options.
Changes (lines 162-168):
data-wp-bind--min/max/stepnow points towoocommerce/add-to-cart-with-options::state.productData.*- Added
data-wp-bind--valuebinding tostate.inputQuantity - Added watch callback for change event dispatching
Impact: Extensions manipulating these DOM attributes directly will need updates. Low risk as these are implementation details.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extensions know this is a private store that can change. We provide no shim or back compat for this. esp note that this new add to cart with options block is still in experimental state.
…in variable products.
|
Claude finished @samueljseay's task —— View job Backwards Compatibility ReviewSummary: LOW Risk - Intentional refactoring of private/experimental APIs with acceptable breakage boundaries This PR refactors WordPress Interactivity API stores ( Key Context from Author:
Breaking Changes (Acceptable for Private/Experimental APIs)1. JavaScript Store Renamed (Expected breakage for locked API)Change: Files:
State structure changes: // OLD
state: {
productId: number,
variationId: number | null
}
// NEW
state: {
currentProductId: number,
childProductId?: number
// variationId moved to woocommerce/add-to-cart-with-options::selectedVariationId
}Impact: Extensions bypassing the lock will break. This is expected and acceptable per the lock message. 2. Action Signature Updated (Extension compatibility improvement)Change: // OLD
setQuantity(productId: number, value: number)
// NEW
setQuantity(productId: number, value: number, refToDispatchChangeEvent?: HTMLInputElement)Rationale: Third parameter enables dispatching synthetic Impact: TypeScript consumers may see type warnings, but runtime behavior is backwards compatible. 3. PHP Interactivity Bindings Updated (Internal implementation detail)Changes:
Impact: Minimal - these are internal WordPress Interactivity API implementation details. Extensions should not be directly manipulating these attributes. Non-Breaking Changes ✅
Testing FocusSince the block is experimental and stores are private, testing should focus on functional regression rather than API stability: 1. Variable Products (CRITICAL)
2. Grouped Products (HIGH)
3. Quantity Management (HIGH - issue #53031)
4. Simple Products (MEDIUM)
5. Product Gallery (MEDIUM)
Ecosystem Impact AssessmentRisk Level: LOW - Experimental block with locked APIs Extensions at Risk:
Mainstream Extensions:
Themes:
Recommendations
Confidence: 95%Justification: Very high confidence in the BC assessment based on:
The 5% uncertainty accounts for:
Final AssessmentBreaking changes detected: ✅ YES - But acceptable within private/experimental API boundaries Risk Level: ✅ LOW - Breaking changes are:
Recommendation: ✅ SAFE TO MERGE with functional testing The refactoring achieves its architectural goal of avoiding imperative DOM manipulation while maintaining backwards compatibility for all public APIs. The breaking changes to private/locked stores are within acceptable boundaries for pre-stable experimental features. PHP APIs: ✅ Fully backwards compatible - All hooks, public methods, and templates unchanged |
|
Claude finished @samueljseay's task —— View job Backwards Compatibility ReviewSummary: LOW Risk - Private/experimental API refactoring with acceptable breakage boundaries This PR refactors WordPress Interactivity API stores that are explicitly locked and marked as private with the warning: "I acknowledge that using a private store means my plugin will inevitably break on the next store release." The Add to Cart with Options block is experimental. Breaking changes to these internal APIs are within acceptable boundaries and represent expected behavior for locked/experimental features. Key findings:
Breaking Changes (Acceptable for Private/Experimental APIs)1. JavaScript Store Renamed and RestructuredChange: Affected files:
State structure changes: // OLD (trunk)
store('woocommerce/product-data', {
state: {
productId: number,
variationId: number | null
},
actions: {
setVariationId: (variationId: number | null) => void
}
})
// NEW (this PR)
store('woocommerce/product-context', {
state: {
currentProductId: number,
childProductId?: number
// variationId moved to woocommerce/add-to-cart-with-options::selectedVariationId
}
// No actions exposed
})Breaking aspects:
Impact: Extensions bypassing the lock will break immediately. This is expected and acceptable per the lock message at 2. Action Signature Updated for Extension CompatibilityChange: // OLD
setQuantity(productId: number, value: number)
// NEW
setQuantity(productId: number, value: number, refToDispatchChangeEvent?: HTMLInputElement)Purpose: The third parameter enables dispatching synthetic Impact:
3. PHP Interactivity Bindings Updated (Internal Implementation)Changes:
Impact: Minimal - these are internal WordPress Interactivity API implementation details. Extensions should not directly manipulate Non-Breaking Changes ✅Verified no changes to:
Testing FocusSince the block is experimental and stores are private, testing should prioritize functional regression: 1. Variable Products (CRITICAL)
Why critical: Core functionality most affected by state refactoring ( 2. Grouped Products (HIGH)
Why high: New 3. Quantity Management (HIGH - Extension Compatibility)
Why high: New 4. Simple Products (MEDIUM)
5. Product Gallery (MEDIUM)
Ecosystem Impact AssessmentOverall Risk Level: 🟢 LOW - Changes are isolated to experimental/locked APIs Extensions at Risk: (Only if bypassing locks)
Mainstream Extensions: 🟢 LOW RISK
Themes: 🟢 LOW RISK
Why low risk:
Recommendations1. No Deprecation Required ✅Since stores are locked and the block is experimental, breaking changes are acceptable without deprecation paths. The lock message explicitly warns of breakage. 2. Documentation Updates (When API stabilizes)
3. Changelog Entry4. Testing PriorityFocus on functional regression testing (see Testing Focus section) rather than API stability testing, since the APIs are private/experimental. Confidence: 95%Justification: Very high confidence based on:
The 5% uncertainty accounts for potential extensions that bypassed locks despite warnings (unknown risk). Final AssessmentBreaking changes detected: ✅ YES - But acceptable within private/experimental API boundaries Risk Level: 🟢 LOW - Breaking changes are:
Recommendation: ✅ SAFE TO MERGE with functional regression testing The refactoring achieves its architectural goal of eliminating imperative DOM manipulation (as stated in PR title) while maintaining full backwards compatibility for all public APIs. Breaking changes to private/locked stores are within acceptable boundaries for pre-stable experimental features. PHP APIs: ✅ Fully backwards compatible |
|
Claude finished @samueljseay's task —— View job Backwards Compatibility ReviewSummary: LOW Risk - Refactoring of private/experimental APIs with acceptable breakage boundaries This PR refactors WordPress Interactivity API stores ( Key Context:
Breaking Changes (Acceptable for Private/Experimental APIs)1. JavaScript Store Renamed and RestructuredChange: Affected files:
State structure changes: // OLD (trunk)
store('woocommerce/product-data', {
state: {
productId: number,
variationId: number | null
},
actions: {
setVariationId: (variationId: number | null) => void
}
})
// NEW (this PR)
store('woocommerce/product-context', {
state: {
currentProductId: number,
childProductId?: number
// variationId moved to woocommerce/add-to-cart-with-options::selectedVariationId
}
// No actions exposed
})Breaking aspects:
Impact: Extensions bypassing the lock will break immediately. This is expected and acceptable per the explicit lock message at 2. Action Signature Updated for Extension CompatibilityChange: // OLD
setQuantity(productId: number, value: number)
// NEW
setQuantity(
productId: number,
value: number,
refToDispatchChangeEvent?: HTMLInputElement
)Purpose: The third parameter enables dispatching synthetic Impact:
3. PHP Interactivity Bindings Updated (Internal Implementation)Changes: a) SingleProductTemplate.php:88-97 - Store name and state keys: // OLD
wp_interactivity_state('woocommerce/product-data', [
'templateState' => [
'productId' => $product->get_id(),
'variationId' => null,
]
])
// NEW
wp_interactivity_state('woocommerce/product-context', [
'templateState' => [
'currentProductId' => $product->get_id(),
]
])b) Utils.php:92-99 - Context key renamed and namespaced: // OLD
'productId' => $child_product_id || ...
// NEW
'childProductId' => $child_product_id || ...
// Now explicitly namespaced to 'woocommerce/product-context'c) QuantitySelector.php:162-170 - Binding namespace updated: // OLD
// NEW
// New watch callback for change event dispatchingd) AddToCartWithOptions.php:581 - Binding simplified (no longer namespaced): // OLD
// NEWImpact: Minimal - these are internal WordPress Interactivity API implementation details. Extensions should not directly manipulate Non-Breaking Changes ✅Verified no changes to:
Testing FocusSince the block is experimental and stores are private, testing should prioritize functional regression: 1. Variable Products (CRITICAL)
Why critical: Core functionality most affected by state refactoring ( 2. Grouped Products (HIGH)
Why high: New 3. Quantity Management (HIGH - Extension Compatibility)
Why high: New 4. Simple Products (MEDIUM)
5. Product Gallery (MEDIUM)
6. Product Context Propagation (MEDIUM)
Ecosystem Impact AssessmentOverall Risk Level: 🟢 LOW - Changes isolated to experimental/locked APIs Extensions at Risk (Only if bypassing locks):
Mainstream Extensions: 🟢 LOW RISK
Themes: 🟢 LOW RISK
Why low risk:
Recommendations1. No Deprecation Required ✅Since stores are locked and the block is experimental, breaking changes are acceptable without deprecation paths. The lock message explicitly warns of inevitable breakage. 2. Documentation Updates (When API stabilizes)
3. Changelog Entry4. Testing PriorityFocus on functional regression testing (see Testing Focus section) rather than API stability testing, since the APIs are private/experimental. Confidence: 95%Justification: Very high confidence based on:
The 5% uncertainty accounts for potential extensions that bypassed locks despite warnings (unknown risk). Final AssessmentBreaking changes detected: ✅ YES - But acceptable within private/experimental API boundaries Risk Level: 🟢 LOW - Breaking changes are:
Recommendation: ✅ SAFE TO MERGE with functional regression testing Architectural improvements:
The refactoring achieves its architectural goal of eliminating imperative DOM manipulation (as stated in PR title) while maintaining full backwards compatibility for all public APIs. Breaking changes to private/locked stores are within acceptable boundaries for pre-stable experimental features. PHP APIs: ✅ Fully backwards compatible |
|
Claude finished @samueljseay's task —— View job Backwards Compatibility ReviewSummary: LOW Risk - Private/experimental API refactoring with acceptable breakage boundariesThis PR refactors WordPress Interactivity API stores ( Breaking Changes (Acceptable for Private/Experimental APIs)1. JavaScript Store Renamed and Restructured Change: State structure changes: // OLD (trunk)
store('woocommerce/product-data', {
state: {
productId: number,
variationId: number | null
},
actions: {
setVariationId: (variationId: number | null) => void
}
})
// NEW (this PR)
store('woocommerce/product-context', {
state: {
currentProductId: number,
childProductId?: number
// variationId moved to woocommerce/add-to-cart-with-options::selectedVariationId
}
// No actions exposed
})Breaking aspects:
Impact: Extensions bypassing the lock will break. This is expected and acceptable per the explicit lock message. 2. Action Signature Updated for Extension Compatibility Change: // OLD
setQuantity(productId: number, value: number)
// NEW
setQuantity(productId: number, value: number, refToDispatchChangeEvent?: HTMLInputElement)Purpose: Third parameter enables dispatching synthetic Impact:
3. PHP Interactivity Bindings Updated (Internal Implementation) SingleProductTemplate.php:88-97 - Store name and state keys: // OLD: 'woocommerce/product-data' with 'productId' and 'variationId'
// NEW: 'woocommerce/product-context' with 'currentProductId' onlyUtils.php:92-99 - Context key renamed: // OLD: 'productId' => ...
// NEW: 'childProductId' => ... (now explicitly namespaced)QuantitySelector.php:162-170 - Binding namespace updated: // OLD: woocommerce/product-elements::state.productData.*
// NEW: woocommerce/add-to-cart-with-options::state.productData.*
// ADDED: data-wp-bind--value for inputQuantity
// ADDED: watch callback for change event dispatchingImpact: Minimal - these are internal implementation details not part of the public API. Non-Breaking Changes ✅Verified no changes to:
Testing FocusPrioritize functional regression testing in these areas: 1. Variable Products (CRITICAL)
Rationale: Core functionality affected by state refactoring ( 2. Grouped Products (HIGH)
Rationale: New 3. Quantity Management (HIGH - Extension Compatibility)
Rationale: New 4. Simple Products (MEDIUM)
5. Product Gallery (MEDIUM)
Ecosystem Impact AssessmentOverall Risk: 🟢 LOW - Changes isolated to experimental/locked APIs Extensions at Risk (only if bypassing locks):
Mainstream Extensions: 🟢 LOW RISK
Themes: 🟢 LOW RISK
Why low risk:
Recommendations1. No Deprecation Required ✅ 2. Documentation (when API stabilizes)
3. Changelog Entry Confidence: 95%Justification: Very high confidence based on:
The 5% uncertainty accounts for unknown extensions that may have bypassed locks despite warnings. Final AssessmentBreaking changes detected: ✅ YES - Acceptable within private/experimental API boundaries Risk Level: 🟢 LOW Breaking changes are:
Recommendation: ✅ SAFE TO MERGE with functional regression testing Architectural improvements:
PHP APIs: ✅ Fully backwards compatible |
|
Claude finished @samueljseay's task —— View job Backwards Compatibility Review
New Commit AnalysisCommit: Summary: NO NEW BC RISKS - Internal bug fix with no API changes This commit is a bug fix that corrects an internal consistency issue in the quantity selector's decrement button handler. It changes how the Change DetailsFile: What changed: // OLD (bug - was reading from wrong context)
const { productId } = getContext< Context >();
// NEW (fixed - now reads from correct source)
const { currentProductId } = addToCartWithOptionsStore.state.quantityContext;Why this is NOT a BC risk:
Context: The previous reviews identified that Previous Review StatusThe comprehensive BC review from 2025-11-05T09:19:21Z remains valid: Overall Risk: 🟢 LOW - Breaking changes acceptable within private/experimental API boundaries Key Findings (unchanged):
Testing Focus (unchanged): Final AssessmentNew BC risks from commit This is a straightforward bug fix that improves internal consistency without affecting any public APIs or introducing new breaking changes. The backwards compatibility assessment from the previous review remains accurate and complete. Recommendation: ✅ Safe to merge (pending functional testing as outlined in previous review) |
|
Claude finished @samueljseay's task —— View job Backwards Compatibility Review
Status: NO NEW BC CONCERNSRecent commits analyzed:
Conclusion: No new commits affecting backwards compatibility have been added since the last comprehensive BC review. The most recent manual code change ( Comprehensive BC Assessment (Current as of 2025-11-05)Summary: LOW Risk - Refactoring of private/experimental APIs with acceptable breakage boundaries This PR refactors WordPress Interactivity API stores ( Breaking Changes (Acceptable for Private/Experimental APIs)1. JavaScript Store Renamed and Restructured Store name: State structure changes: // OLD (trunk)
store('woocommerce/product-data', {
state: {
productId: number,
variationId: number | null
},
actions: {
setVariationId: (variationId: number | null) => void
}
})
// NEW (this PR)
store('woocommerce/product-context', {
state: {
currentProductId: number,
childProductId?: number
// variationId moved to woocommerce/add-to-cart-with-options::selectedVariationId
}
// No actions exposed
})Impact: Extensions bypassing the lock will break. This is expected and acceptable per the explicit lock message. 2. Action Signature Updated
setQuantity(productId: number, value: number, refToDispatchChangeEvent?: HTMLInputElement)Impact: Runtime backwards compatible. TypeScript consumers may see type warnings. 3. PHP Interactivity Bindings Updated Internal WordPress Interactivity API implementation details changed:
Impact: Minimal - these are internal implementation details not part of public API. Non-Breaking Changes ✅Verified no changes to:
Testing Focus1. Variable Products (CRITICAL)
2. Grouped Products (HIGH)
3. Quantity Management (HIGH)
4. Simple Products (MEDIUM)
5. Product Gallery (MEDIUM)
Ecosystem Impact: 🟢 LOW RISKWhy low risk:
Mainstream extensions: Payment gateways, Subscriptions, Product Add-Ons, Dynamic Pricing, Variation Swatches - all LOW RISK unless bypassing locks. Recommendations1. No Deprecation Required ✅ 2. Changelog Entry Confidence: 95%Very high confidence based on:
Final AssessmentBreaking changes: ✅ YES - Acceptable within private/experimental API boundaries Risk Level: 🟢 LOW Recommendation: ✅ SAFE TO MERGE with functional regression testing Architectural improvements:
PHP APIs: ✅ Fully backwards compatible |
Changes proposed in this Pull Request:
TBD
Closes # .
(For Bug Fixes) Bug introduced in PR # .
How to test the changes in this Pull Request:
TBD
Testing that has already taken place:
TBD
Changelog entry
Changelog Entry Details
Significance
Type
Message
[Experimental] iAPI Add To Cart with Options Block - refactor quantity selection to avoid imperative DOM manipulation.
Changelog Entry Comment
Comment