-
Notifications
You must be signed in to change notification settings - Fork 35
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix policy modal #1458
fix policy modal #1458
Conversation
@@ -27,4 +27,7 @@ export interface PolicyModalProps { | |||
type: PolicyType; | |||
} | |||
|
|||
export const PolicyModal = createRemoteReactComponent('PolicyModal'); | |||
export const PolicyModal = createRemoteReactComponent< |
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 have honestly no idea how/why this worked before 馃
We never combined the Component with its types. We're now passing the props so that the type is complete.
@@ -3,7 +3,8 @@ | |||
"compilerOptions": { | |||
"outDir": "build/ts", | |||
"rootDir": "src", | |||
"jsx": "react-jsx" | |||
"jsx": "react-jsx", | |||
"stripInternal": true |
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.
Missed this when moving back to unstable. We had in internal already. https://github.com/Shopify/ui-extensions/pull/1399/files#diff-62ec3f3adce17523e1588b957e8483dbc86b81ab9b6062022b35c69880faa6b2R6
But not for the non-react package. I've also added it here. I don't think it makes sense to only strip internals for react.
1f85d07
to
1fb524f
Compare
Background
Not stripping the internal fields caused clashes in an app.
I missed moving this over. We already had this setting in internal: https://github.com/Shopify/ui-extensions/pull/1399/files#diff-256b5258d7b08eedfba647de7d28bae26f13f08d985b93f5c1ddb456644857d2R7
I also changed how the type is constructed. I have honestly no idea how/why this worked before 馃
We never combined the Component with its types. We're now passing the props so that the type is complete.
馃帺 will be done in said app, after merge.
Checklist