-
Notifications
You must be signed in to change notification settings - Fork 672
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 theme-aware Global component #2385
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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 ac36c53:
|
I'll take a look at those tests. |
packages/global/src/index.tsx
Outdated
jsx(EmotionGlobal, { | ||
styles: (emotionTheme: unknown) => { | ||
const theme = emotionTheme as Theme | ||
return css(sx)(theme) |
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'm kinda wondering if
useMemo(() => css(sx)(theme), [sx, theme])
here would be an overkill.
We can always add it later after we know more I suppose.
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.
Oh good call. I wish evaluating perf with hooks like that with were easier…conceptually makes sense to use it at least
examples/next/lib/theme.ts
Outdated
@@ -1,6 +1,6 @@ | |||
import { makeTheme } from '@theme-ui/css/utils' | |||
// import { makeTheme } from '@theme-ui/css/utils' |
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.
After running pnpm build
Next.js dev server doesn't catch that the libraries are built instantly, so I removed .next
dir in the example, restarted pnpm dev
and the error on this import went away.
The error was basically saying we can't import TypeScript files directly, and that's totally fine, because we want to develop examples against prebuilt libraries, but even after rebuilding you gotta clean the cache if you started dev server previously.
I've sent a PR against your branch with a change motivated by the failing test here. — #2386. |
…not be named without a reference"
🚀 PR was released in |
At long last, we no longer have to recommend using Emotion's Global component if you need page-specific global CSS. The API:
@hasparus: While it's working fine in projects, in the test environment, I'm getting scoped class names from Emotion, which is causing the tests to fail. I've commented those out, but if you look at the snapshot you can see what's happening—any ideas appreciated so it has full test coverage. Both Emotion's tests & MUI's tests somehow don't run into this issue; I can't tell what's different in our environment.
Version
Published prerelease version:
v0.15.5-develop.1
Changelog
🎉 This release contains work from a new contributor! 🎉
Thank you, Nischal Shakya (@Nischal2015), for all your work!
🐛 Bug Fix
🏠 Internal
useMDXComponents
import #2370 (@Nischal2015)Authors: 3