-
-
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
[WIP] Use Hooks #967
[WIP] Use Hooks #967
Conversation
I made a codesandbox to demonstrate the usage of hooks with emotion 10 : https://codesandbox.io/s/3ykkxov026 Thanks for the advices @mitchellhamilton ! 🥇 |
type Props = { theme: Object } | ||
|
||
// should we change this to be forwardRef/withCSSContext style so it doesn't merge with props? | ||
// should we remove this altogether and tell people to useContext |
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 should! :) although we could simplify it for the users and provide
const useTheme = () => useContext(ThemeContext)
not sure what are ur plans regarding v10 timeline, but i agree with sentiment here reduxjs/react-redux#1065 (comment) |
Hey, guys! import { useEmotion } from '@emotion/hook';
export const Component = () => {
const { css, cx, theme, injectKeyframes, injectGlobal } = useEmotion()
const container = css({ width: '100%', padding: theme.spaces[1] })
const fade = injectKeyframes({
from: { opacity: 0 },
to: { opacity: 1 }
})
return (
<div className={container}></div>
)
}
I could work on it if you are interested. |
Why the jsx pragma is a problem for you? Im curious - would be great to know people problems to potentially solve them. As to the hooks - what would be the advantage of this over just importing "bare" emotion and using it in render? |
Some tools (like fastpack) do not support jsx pragma. Flow had also problems with it, not sure about now. The advantage and the ability to connect with react context to achieve caching and SSR. As far as I understand bare import is not just work in v10. Also it would be really good to make injectKeyframes and injectGlobal destroyable. |
jsx pragma looks to me like a workaround and it still adds a noise in dev tools like styled. |
Preview Docs Built with commit 7e9b819 |
# Conflicts: # .flowconfig # .flowconfig-ci # .github/ISSUE_TEMPLATE.md # package.json # packages/core/src/global.js # packages/core/src/jsx.js # packages/styled-base/src/index.js # yarn.lock
🦋 Changeset is good to goLatest commit: 6af574b We got this. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
packages/core/src/global.js
Outdated
} | ||
cache.insert(``, serialized, sheet, false) | ||
}, | ||
[cache, serialized] |
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.
previous version was only updating styles on serialized.name
change, I'm not sure if the previous version was safe - because I don't quite understand the cursor/keyframes thing and what guarantees it gives in regards to serialized.name
. Would love to learn about it though, could you explain this a little bit?
If we can't put serialized.name
in the inputs array then we can remove this array altogether, as serialized
changes with each render - so technically this input array doesn't give us anything right now
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.
If serialized.name
is the same, the style is the same since the hash of the contents of the keyframes is in the style it's part of the whole styles' hash.
# Conflicts: # packages/core/package.json # packages/emotion-theming/package.json # packages/native/package.json # packages/primitives-core/package.json # packages/styled-base/package.json # packages/styled-base/src/index.js # packages/styled/package.json
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 6af574b:
|
Codecov Report
|
* Tests are broken :( but it works * Add displayName * Use ref * Remove stuff * Update react-test-renderer * Use useContext in more places * Fix a Global insertion order bug * Add a test * Fix typo * Remove passing the theme to the css prop, Global and ClassNames * stuff * stuff * changes * Add a test back * Fix a thing * Update stuff * Update things * Drop support for innerRef entirely * Set correct peerDeps on react + upgrade react-related devDeps * Fix tests * Fix linting error * Remove custom useContext flow types * Fix flow error * Fix layour effect input array to include serialized.name instead of whole serialized * Add a changeset * Add another changeset
What:
Use hooks for stuff
Why:
There's a slight performance and size improvement(~150 bytes). I also have some ideas on how hooks could help some things so I want to do this so we can use hooks.
How:
Checklist:
Not gonna merge this until hooks are stable but just wanted to open this to show some people