fix stripe_web: store card element div globally to prevent mounting errors#2346
fix stripe_web: store card element div globally to prevent mounting errors#2346hifiaz wants to merge 1 commit intoflutter-stripe:mainfrom
Conversation
The card element div was being recreated on each mount, causing issues with Stripe's element mounting. Store it globally and clear on dispose to ensure consistent mounting and unmounting.
📝 WalkthroughWalkthroughModified the Stripe card field widget to cache the card container element and defer mounting the card element until after the container reference is established, updating the disposal logic to clear the cached reference. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/stripe_web/lib/src/widgets/card_field.dart`:
- Line 14: The top-level variable _cardElementDiv is shared across
WebStripeCardState instances and races between the HtmlElementView factory,
initStripe's post-frame callback, and dispose; change _cardElementDiv from a
file-scope variable into an instance field on WebStripeCardState so each state
owns its own HTMLDivElement, update the HtmlElementView factory to use a unique
per-instance viewType key that the factory closure captures (so it writes the
correct instance field), and adjust initStripe (the post-frame callback) and
dispose to reference the instance field instead of the top-level one to avoid
cross-instance clobbering or nulling.
- Around line 90-93: When the early-return checks _cardElementDiv for null (in
the CardField widget where _cardElementDiv is referenced), add a diagnostic log
using the existing dart:developer import instead of silently returning; update
the conditional in the method that contains the snippet (search for
_cardElementDiv and the surrounding code in CardField/State) to call log() (or
log with a level/message) describing that the container is null and why this
might occur before returning, so callers/debuggers see a clear diagnostic when
the element is missing.
|
|
||
| import '../../flutter_stripe_web.dart'; | ||
|
|
||
| web.HTMLDivElement? _cardElementDiv; |
There was a problem hiding this comment.
Global _cardElementDiv is unsafe during concurrent instances or interleaved navigation transitions
_cardElementDiv is a top-level file-scope variable shared by every WebStripeCardState instance. Two code paths race on it:
| Event | Effect on _cardElementDiv |
|---|---|
Platform view factory (per HtmlElementView render) |
writes the newly-created div |
Inner addPostFrameCallback in initStripe |
reads it to mount the Stripe element |
dispose() |
nulls it |
If a second WebCardField is alive at the same time (e.g., the old and new screen overlap on the navigation stack before dispose fires), the factory for the second instance overwrites the global, and the first instance's deferred callback mounts to the wrong container. Conversely, if dispose on the departing widget runs after the new widget's factory has already written the reference, _cardElementDiv = null at Line 197 wipes the freshly valid reference, causing the new instance's inner callback to silently return without mounting (the null-guard at Line 91 swallows the failure).
The pre-existing #card-element CSS selector carried the same single-instance constraint (duplicate IDs), but the global-variable approach is more fragile because the null-out and the factory write are not co-located and can interleave across lifecycle calls.
Recommended fix – promote _cardElementDiv to an instance field and use a per-instance view type key so the factory closure can capture the correct state:
♻️ Proposed refactor
// Remove the top-level variable:
- web.HTMLDivElement? _cardElementDiv;In WebStripeCardState:
class WebStripeCardState extends State<WebCardField> with CardFieldContext {
CardEditController get controller => widget.controller;
+ web.HTMLDivElement? _cardElementDiv; // instance-scoped
+ // Unique per-instance viewType avoids factory-collision between instances.
+ String get _viewType => 'stripe_card_${hashCode}';
`@override`
void initState() {
ui.platformViewRegistry.registerViewFactory(
- 'stripe_card',
- (int viewId) {
- final element = web.HTMLDivElement()..style.border = 'none';
- _cardElementDiv = element;
- return element;
- },
+ _viewType,
+ (int viewId) {
+ final element = web.HTMLDivElement()..style.border = 'none';
+ _cardElementDiv = element;
+ return element;
+ },
);- child: const HtmlElementView(viewType: 'stripe_card'),
+ child: HtmlElementView(viewType: _viewType), `@override`
void dispose() {
detachController(controller);
element?.unmount();
- _cardElementDiv = null;
+ _cardElementDiv = null; // clears instance field only
super.dispose();
}Also applies to: 61-65, 90-93, 197-197
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/stripe_web/lib/src/widgets/card_field.dart` at line 14, The
top-level variable _cardElementDiv is shared across WebStripeCardState instances
and races between the HtmlElementView factory, initStripe's post-frame callback,
and dispose; change _cardElementDiv from a file-scope variable into an instance
field on WebStripeCardState so each state owns its own HTMLDivElement, update
the HtmlElementView factory to use a unique per-instance viewType key that the
factory closure captures (so it writes the correct instance field), and adjust
initStripe (the post-frame callback) and dispose to reference the instance field
instead of the top-level one to avoid cross-instance clobbering or nulling.
| final container = _cardElementDiv; | ||
| if (container == null) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Silent failure on null container — add a diagnostic log
When _cardElementDiv is null (e.g., due to the race described above, or a frame-ordering edge case), the early return leaves the card field blank with no indication of failure. The file already imports dart:developer for exactly this purpose.
final container = _cardElementDiv;
if (container == null) {
+ dev.log(
+ 'WebCardField: card container element is null; Stripe card element not mounted.',
+ name: 'stripe_web',
+ level: 900, // Level.WARNING
+ );
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| final container = _cardElementDiv; | |
| if (container == null) { | |
| return; | |
| } | |
| final container = _cardElementDiv; | |
| if (container == null) { | |
| dev.log( | |
| 'WebCardField: card container element is null; Stripe card element not mounted.', | |
| name: 'stripe_web', | |
| level: 900, // Level.WARNING | |
| ); | |
| return; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/stripe_web/lib/src/widgets/card_field.dart` around lines 90 - 93,
When the early-return checks _cardElementDiv for null (in the CardField widget
where _cardElementDiv is referenced), add a diagnostic log using the existing
dart:developer import instead of silently returning; update the conditional in
the method that contains the snippet (search for _cardElementDiv and the
surrounding code in CardField/State) to call log() (or log with a level/message)
describing that the container is null and why this might occur before returning,
so callers/debuggers see a clear diagnostic when the element is missing.
1 participant
The card element div was being recreated on each mount, causing issues with Stripe's element mounting. Store it globally and clear on dispose to ensure consistent mounting and unmounting.
Summary by CodeRabbit