[react-jss] Big Refactor #949
Conversation
* Update to new react context for the jss context * Fixed types * Fix some tests * Remove unnecessary cloning * Fix JssProvider * Update logic of merging context * Fix a few issues and tests * Make getTheme non public * Update warning inside JssProvider * Remove duplicate beforeEach * Update size-snapshot * Fix a few bugs * Created a Managers type * Create sheetOptions prop type definition * Removed meta property from sheet options shape * Fix creating a new generateId for every render * Move media into it's own prop * Added changelog * Rename to JssContextSubscriber * Moved context to jssContext prop instead of spreading it
* Update to new react context for the jss context * Fixed types * Fix some tests * Remove unnecessary cloning * Fix JssProvider * Update logic of merging context * Fix a few issues and tests * Make getTheme non public * Update warning inside JssProvider * Remove duplicate beforeEach * Update size-snapshot * Fix a few bugs * Remove inject option * Fix * Always inject the theme * Only inject theme when injectTheme is true
* Add forwardRef support * Add support for merging the classes * Fix path of test file * Remove jsdoc comment * Implement custom memoize-one * Convert function to arrow function
docs/react-jss.md
Outdated
| In case you need to access the theme but not render any CSS, you can also use `withTheme`. It is a Higher-order Component factory which takes a `React.Component` and maps the theme object from context to props. [Read more about `withTheme` in `theming`'s documentation.](https://github.com/iamstarkov/theming#withthemecomponent) | ||
| #### Accessing the theme inside the styled component | ||
|
|
||
| Pass the `injectTheme` option to `withStyles` so your theme will be injected into your wrapped component. |
There was a problem hiding this comment.
Arn't we injecting the theme anyways if styles is a function?
There was a problem hiding this comment.
No only if injectTheme is true
There was a problem hiding this comment.
Oh I got confused by the example, it shows the theme being injected in to styles function but doesn't show it in the react component which made me assume we need injectTheme to pass it to the styles function.
theme is getting passed to the styles function always! If no that would be evtl a problem.
theme is not getting passed with props to the user component by default, for this one needs injectTheme option, right? we need to make that difference clear in the docs if technically we are on the same page.
| theming?: Theming<Theme>, | ||
| inject?: Array<'classes' | 'themes' | 'sheet'>, | ||
| injectTheme?: boolean, | ||
| jss?: Jss | ||
| } & StyleSheetFactoryOptions |
There was a problem hiding this comment.
I would prefer to use spread operator instead.
There was a problem hiding this comment.
yeah, I use spread currently most of the time, btw they do exactly the same thing, right?
There was a problem hiding this comment.
Not quite. Disjunction has own use cases. Not sure how it work in this case. For example before 0.90 we may get non-nullable shape via $Shape<T> | {}.
| @@ -0,0 +1,259 @@ | |||
| // @flow | |||
| import React, {Component, type ComponentType, type Node} from 'react' | |||
There was a problem hiding this comment.
I personally prefer import * as React from 'react';. This is obvious end of react transition to esm. However we will be able to achieve this only in react v19 in the best way.
There was a problem hiding this comment.
Why? Is it just taste? I prefer to always use specific named imports instead of an object that has everything. Shouldn't this get us some optimizations once react has esm bundle?
There was a problem hiding this comment.
Nope. Rollup expands namespaces with usages, so in the end you see the same expanded set of named imports.
With default export you get a bloat with whole react api object.
And named exports are more natural in context of react stuff. Default exports is again a legacy of cjs.
There was a problem hiding this comment.
Can you make an example please? I didn't get it.
There was a problem hiding this comment.
import * as React from 'react';
React.Component;
React.createContext;is expanded into
import {Component, createContext} from 'react';
Component;
createContext;This is super nice optimisation feature of rollup. Webpack don't do this and instead duplicates React.createElement everywhere which increase bundle size a lot. Guys invents babel/webpack plugins for this.
For me it's just absurd. Webpack makes me said last months. It has fast enough development mode but production is far from perfect.
There was a problem hiding this comment.
I personally don't use default export. But I ask only to stop import react as default to reduce bundle size and simplify future transition.
There was a problem hiding this comment.
What exactly is the impact in bundle size when default export is used?
There was a problem hiding this comment.
React.default.createElement instead of just createElement for example. Doesn't solve the problem for webpack though.
There was a problem hiding this comment.
This size is not going away with uglify?
There was a problem hiding this comment.
Perhaps something like unsafeGetters option will help. Or may broke something.
* [WIP] Update to new react context for the jss context (cssinjs#924) * Update to new react context for the jss context * Fixed types * Fix some tests * Remove unnecessary cloning * Fix JssProvider * Update logic of merging context * Fix a few issues and tests * Make getTheme non public * Update warning inside JssProvider * Remove duplicate beforeEach * Update size-snapshot * Fix a few bugs * Created a Managers type * Create sheetOptions prop type definition * Removed meta property from sheet options shape * Fix creating a new generateId for every render * Move media into it's own prop * Added changelog * Rename to JssContextSubscriber * Moved context to jssContext prop instead of spreading it * [react-jss] Remove inject option (cssinjs#934) * Update to new react context for the jss context * Fixed types * Fix some tests * Remove unnecessary cloning * Fix JssProvider * Update logic of merging context * Fix a few issues and tests * Make getTheme non public * Update warning inside JssProvider * Remove duplicate beforeEach * Update size-snapshot * Fix a few bugs * Remove inject option * Fix * Always inject the theme * Only inject theme when injectTheme is true * Upgrade theming package (cssinjs#942) * Add forwardRef support (cssinjs#943) * [react-jss] Merge classes instead of overwriting (cssinjs#946) * Add forwardRef support * Add support for merging the classes * Fix path of test file * Remove jsdoc comment * Implement custom memoize-one * Convert function to arrow function * Fix * Update some of the tests * Update dynamic styles tests * Fix SSR tests * Add theme inject tests * Add merge classes tests * Remove dependency of react-dom * Fix an issue with memoizing and memoize the context * Remove createHoC file and remove injectSheet to withStyles * Update TS types * Update TS types * Fix flow types * Fix react-jss types * Rename injectSheet to withStyles in docs * Fix react-jss types * Update changelog.md * Rename class and * Fix tests * Fix a few typos in docs * Fix changelog * Use javascript for codeblocks instead of js * Upgrade theming * Fix type import * Update size-snapshot * Fix docs * Update size-snapshot * Upgrade theming to v3.0.2 * Use travis_wait for test command * Remove travis_wait command * Update docs
What would you like to add/fix?
Corresponding issue (if exists):