-
Notifications
You must be signed in to change notification settings - Fork 645
(don't merge) See diff for #2419 #2421
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
Conversation
|
lib-esm/ActionList/Description.js
Outdated
| import React from 'react'; | ||
| import Box from '../Box.js'; | ||
| import '../sx.js'; | ||
| import '@styled-system/css'; |
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.
this feels like a strange replacement?
also looks like import '../sx.js' was imported without any import specifiers until now, strange?
size-limit report 📦
|
lib-esm/Avatar.js
Outdated
| var Avatar$1 = Avatar; | ||
|
|
||
| export { Avatar$1 as default }; | ||
| export { Avatar as default }; |
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's a bunch of removed $1 which honestly i like that
lib/index.js
Outdated
| exports.themeGet = constants.get; | ||
| exports.BaseStyles = BaseStyles; | ||
| exports.ThemeProvider = ThemeProvider["default"]; | ||
| exports.ThemeProvider = ThemeProvider.ThemeProvider; |
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.
as long as these 2 are the same, all good!
| @@ -0,0 +1,15 @@ | |||
| 'use strict'; | |||
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.
nice, this is the file we want to ship
| const TYPOGRAPHY = compose(styledSystem__namespace.typography, whiteSpace); | ||
| // Border props | ||
| compose(styledSystem__namespace.border, styledSystem__namespace.shadow); | ||
| const BORDER = compose(styledSystem__namespace.border, styledSystem__namespace.shadow); |
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.
this output is wild? compose used to called without assigning to anything, that's strange?!
i guess this is a bugfix 😅
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 think what happens when BORDER is removed is that it considers compose and if it is pure. Looking at styled-system, it doesn't have sideEffects listed so it can't assume compose is pure and has to leave it in case of side-effects.
| const LAYOUT = styledSystem.layout; | ||
|
|
||
| export { COMMON, TYPOGRAPHY, get }; | ||
| export { BORDER, COMMON, LAYOUT, TYPOGRAPHY, get }; |
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.
it looks like border and layout were not exported till now? is it because they were tree shaken?
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 think so, it seemed like they aren't re-exported through src/index.ts and were removed based on usage in the codebase
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.
yay it's fixed!
| @@ -0,0 +1,13 @@ | |||
| // JSDOM doesn't mock ResizeObserver | |||
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.
nice, this is the file we want to ship (lib-esm edition)
… into fix/update-utils-entrypoint
| @@ -0,0 +1,12 @@ | |||
| export { useOnOutsideClick } from './useOnOutsideClick.js'; | |||
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.
this is a big win! love it
|
job done, let's ship |
Uh oh!
There was an error while loading. Please reload this page.