-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add Instances #464
Add Instances #464
Conversation
packages/create-emotion/src/sheet.js
Outdated
sheet: string[] | ||
tags: HTMLStyleElement[] | ||
nonce: string | void | ||
constructor(nonce?: string) { |
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.
Should we make this an object in case we need more options later.
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 don't think we need to since it's an internal API.
* Try implementing streaming * More ideas that I didn't test * Add deps and more stuff to extractCritical * More stuff * I'm still totally guessing * Fixes, update react and start tests for streaming ssr * It's a start * More stuff * Another thing I haven't tested * Simplify extractCritical * Lots of stuff * Increase bundlesize * Change hydration and document new APIs * Remove check for case that cannot happen * Remove names cache * Fix some tests * Fix more tests * Fix streaming tests * Add create-emotion-server package * So close * I don't know how it worked before * Increase bundlesize * Fix dist tests Closes #341
what is missing for this to get merged? i cant wait to start using renderToNodeStrean with emotion |
* Start of jest-emotion-react based on jest-glamor-react * Update snapshots * Use inserted cache instead of StyleSheet to get styles so it works with jsdom * Throw better error when css.parse throws * Add test with media query before other rule I've already fixed this but I want to commit the test without the fix so I know it does actually fail without the fix * Fix stuff and other stuff * Replace all css- classes like kentcdodds/jest-glamor-react#28 * More stuff * Rename to jest-emotion, make createSerializer a named export, add getStyles so people can build other tools on it * Update jest configs * Update snapshot
* Start key and container options * Fix stuff * Increase bundlesize * Add another test * Add another test * Update README.md
Codecov Report
|
@@ -9,10 +9,15 @@ | |||
"^emotion-utils$": "<rootDir>/packages/emotion-utils/dist/index.cjs.js", | |||
"^emotion-server$": "<rootDir>/packages/emotion-server/lib", | |||
"^emotion-theming$": "<rootDir>/packages/emotion-theming/dist/index.cjs.js", | |||
"^babel-plugin-emotion": "<rootDir>/packages/babel-plugin-emotion/lib" | |||
"^babel-plugin-emotion": "<rootDir>/packages/babel-plugin-emotion/lib", |
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.
@mitchellhamilton would you be interested in using a package that would create those for you? I've created some time ago https://github.com/Andarist/lerna-alias for my use case, but can easily add an option to use main
field / standard node resolve algo
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 we might be able to remove these mappings since it should just work with lerna/yarn linking but I just haven't tried. I looked at lerna-alias before and I tried to use it for the rollup config but it wasn't working, I can't remember exactly why though. I think it would be really good for the main jest config though.
} | ||
|
||
class ThemeProvider extends Component<Props> { | ||
getTheme: ThemeProvider.prototype.getTheme |
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.
had no idea a type can be specified like this, is there reason why it's defined like this and why it's defined at all? i mean - methods should be inferrable from the class definition, right?
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.
Flow complains without it because of the bind in the constructor https://flow.org/try/#0PQKgBAAgZgNg9gdzCYAoVBjGBDAzrsAFQAsBLAOwHMwBvVMMDOc3AFwCcBXDVudgCgCUtemFZlcAOlxwAtgFMAsvPFwAJmAC8YidLlKVxdZIBGFNf3Glcg0QF9RMhctUXhdBvdR2gA
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.
wouldnt transform-class-properties
resolve this?
|
||
outerTheme: Object | ||
broadcast: * | ||
unsubscribeToOuterId: number |
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 not entirely true, the correct type would be ?number
although making it so would probably cause flow complain before using this and would force a runtime check 🤔
What:
Add three new packages:
create-emotion
,create-emotion-styled
andcreate-emotion-server
.create-emotion
exports a function which accepts acontext
, this should generally be Node'sglobal
or the browser'swindow
and other options likenonce
and maybestylisPlugins
(I'm not sure if we should removeuseStylisPlugin
and use an option, keep both or keep usinguseStylisPlugin
, I'm leaning towards an option since stylis plugins are behind the caches so it could be confusing because if you add a plugin withuseStylisPlugin
and callcss
with something that is cached, it won't run the plugin). It returns an object that contains all the current exports of emotion.create-emotion-styled
exports a function which accepts an emotion instance (fromcreate-emotion
) and options for thechannel
used for theming, thecontextTypes
for theming,createElement
(a React-likecreateElement
) andComponent
(a React-likeComponent
). It returns astyled
API, likereact-emotion
exports.create-emotion-server
(I haven't done this yet as I'm waiting to finish #448 first)There will also be some new options to
babel-plugin-emotion
to specify the paths to emotion instances and the primary emotion path that will be used to add imports for the css prop (and potentially other things in the future).This will also store the caches on globals for the
emotion
package so that #349 is solved and loading the same instance twice will use the same caches. This mainly affects SSR where there are often multiple instances ofemotion
in different dependencies and they're not deduplicated by webpack.This does not affect people who currently use
emotion
,react-emotion
,preact-emotion
andemotion-server
and don't need instances. (except that loading the modules twice will use the same caches so #349 will be fixed)Why:
Closes #430
Closes #403
Closes #349
How:
Checklist: