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

feat(core): CHECKOUT-4909 Pass in attributes to stylesheet #11

Merged
merged 1 commit into from
May 25, 2020

Conversation

vmparra
Copy link
Contributor

@vmparra vmparra commented May 22, 2020

What?

Pass in attributes to stylesheet

Why?

To be able to pass in attributes such as integrity and crossorigin

Testing / Proof

  • Unit

@bigcommerce/frontend @bigcommerce/checkout

@@ -4,12 +4,17 @@ import BrowserSupport from './browser-support';

export interface LoadStylesheetOptions {
prepend: boolean;
attributes: StylesheetAttributes;
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be marked as optional?

Copy link
Contributor Author

@vmparra vmparra May 25, 2020

Choose a reason for hiding this comment

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

I believe is mandatory, prepend won't serve any purpose if we don't provide actual attributes. Please see how script loader does it Script-loader#7

Copy link
Contributor

Choose a reason for hiding this comment

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

I think prepend is not related to the attributes itself, but rather, where it's loaded?

https://github.com/bigcommerce/script-loader-js/pull/11/files#diff-fad3242aa36c1c27d88b5b7c6a77e7f1R49

Copy link
Contributor

Choose a reason for hiding this comment

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

I checked Script-loader#7 but I think it's probably a typescript bug. Should be marked as optional there too, as there is code that caters for the scenario when attributes is missing.

@vmparra vmparra added ready to merge Pull request that have been approved and are waiting to be merged reviewed and removed reviewable labels May 25, 2020
@vmparra
Copy link
Contributor Author

vmparra commented May 25, 2020

@capsula4 I've addressed your comments ♻️

@lpschz lpschz merged commit 47ee685 into bigcommerce:master May 25, 2020
@vmparra vmparra removed ready to merge Pull request that have been approved and are waiting to be merged sydney labels May 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants