Skip to content
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

Include stripe.js for fraud resistance (MNTOR-1769) #4227

Merged
merged 3 commits into from
Feb 20, 2024
Merged

Include stripe.js for fraud resistance (MNTOR-1769) #4227

merged 3 commits into from
Feb 20, 2024

Conversation

flozia
Copy link
Collaborator

@flozia flozia commented Feb 19, 2024

References:

Jira: MNTOR-1769

Description

Include stripe.js for fraud resistance as requested in MNTOR-1769.

How to test

Make sure <script src="https://js.stripe.com/v3"></script> is present and stripe.js is being loaded without any CSP errors.

@flozia flozia requested review from rhelmer and Vinnl February 19, 2024 11:50
@flozia flozia self-assigned this Feb 19, 2024
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

"use client";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain why this needs to be a client component, and why it can't just be imported directly in the layout?

Copy link
Collaborator Author

@flozia flozia Feb 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opted for including stripe.js as a module, this is also how MDN does it.

An alternative would be to directly add <script src="https://js.stripe.com/v3" async></script>. When it’s imported as a side effect, like it is in this PR, the <script> is being injected client side and expects the DOM to be present — thus the "use client".

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But client components are still pre-rendered on the server, so there might still not be a DOM present. Possibly you need to do a dynamic import("@stripe/stripe-js") inside a useEffect to make it only run on the client side?

Copy link
Collaborator Author

@flozia flozia Feb 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is true. As is it would run on the server, but it is not leading to an error since this is being dealt with by the script. It is definitely better to make the component import it client-only as suggested — updated in 76d6c81.

Copy link
Collaborator

@Vinnl Vinnl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@flozia flozia merged commit 50fe488 into main Feb 20, 2024
13 checks passed
@flozia flozia deleted the mntor-1769 branch February 20, 2024 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants