-
Notifications
You must be signed in to change notification settings - Fork 115
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: tree-shaking in blade components #1045
Changes from 2 commits
d29f01e
601489f
d7fe2a5
5c08c14
839a9c8
6fca7bb
31770b3
dae2761
dd61197
0781b27
9772fbe
da5f6d6
4cda6f6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -101,7 +101,8 @@ const _StyledBaseButton: React.ForwardRefRenderFunction<BladeElementRef, StyledB | |
); | ||
}; | ||
|
||
const StyledBaseButton = React.forwardRef(_StyledBaseButton); | ||
StyledBaseButton.displayName = 'StyledBaseButton'; | ||
const StyledBaseButton = /*#__PURE__*/ Object.assign(React.forwardRef(_StyledBaseButton), { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So here, what is the main factor? The PURE comment or the Object.assign? which one does the magic? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The PURE comment. You can also define component inside a function and add prop inside that and return the component with imediate function call and this would probably work. const StyledBaseButton = /*#__PURE__*/ (() => {
const MyComponent = () => {};
MyComponent.displayName = 'MyComponent';
return MyComponent;
})(); The PURE comment only works on function calls so |
||
displayName: 'StyledBaseButton', | ||
}); | ||
|
||
export default StyledBaseButton; |
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.
We can also add the explanation as comment to one particular component (probably button)
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.
We might need a lint rule to enforce this, easy to miss out. How did you even figure this out lol 馃樃
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.
Btw is there a babel plugin that automatically does this transformation? Seems like a common issue libraries might be running into 馃
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.
There is this babel plugin - https://github.com/mbrowne/babel-plugin-pure-static-props. I tried this first but it didn't work. I think it works when props are on react component and not when they are on refs. Even for react components it didn't work at lots of places for some reason. Couldn't find any other plugin either.
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.
Yes definitely! Exploring if it can be eslint rule or babel plugin.
Lol it was tricky