Comments (13)
from material-ui.
Agreed. We'll have to clear these issues in our docs so that anything that goes into the config file should not be part of the code that goes into the source code. I've made a note of that.
from material-ui.
I wouldn't say it's a bug, rather a lack of documentation on how to export themes and styled
/css
function exports.
When you are using the same package to export styled/css
calls as well as themes, either of those, preferably theme
should be exported on a subpath and not in the same export as others.
In your case, you should be exposing your theme
on @monorepo/pkg2/theme
and other runtime exports can be directly on @monorepo/pkg2
. This is becasue when trying to import @monorepo/pkg2
in your vite.config
, it is indirectly also executing the css
function which would throw error because it requires a code transform to work.
Hope this is clear. Let me know if this worked.
from material-ui.
I disagree. The logic within Pigment's API is too fragile. css
, keyframes
, etc. all always
throw an error when invoked. Instead, they should throw an error only in development and only in browser environment.
You are welcome to close the issue though, if you believe the DX is safe enough.
Anything cannot be safely imported from the file with a theme.
- if you import
a value
or evena type
, you risk the file you import references a common file with a file that uses pigment
a lack of documentation on how to export themes
The issue has nothing to do with the documentation, it's about how the vite plugin fails
(it says you didn't configure the bundler). At least the plugin should fail, stating there's a reference of a pigment file when importing a theme.
The simple solution would be to check on the existence of window
, when Pigment's API is invoked.
- there could be a more complex check whether Pigment is invoked in browser or during bundling
from material-ui.
I am not sure where the fragility comes from. Importing a file in your bundler config is different than importing the same in your source code. Imports in source code go through transforms configured in the Vite config plugins. The same is not true for imports in config files.
When you are importing -
import { theme } from "../pkg2/src";
Vite executes the pkg2/src/index.ts
file. This file also imports your pigment.css()
definition. So this function gets executed as well. If you look at the css
function definiton, it simply throws an error:
function css() {
throw new Error(
`@pigment-css/react: You were trying to call "css" function without configuring your bundler. Make sure to install the bundler specific plugin and use it. @pigment-css/vite-plugin for Vite integration or @pigment-css/nextjs-plugin for Next.js integration.`
);
}
This is because it is supposed to be compiled away during bundling and get replaced with string class name. So importing and calling the same css
function in your vite config file will throw the error since there are no code transforms involved.
This is the same reason that the extendTheme
function is available through the @pigment-css/vite-plugin
package and not from the @pigment-css/react
package.
You can try this patch in your above repo. It just removes the theme export from the main index file.
commit a75173dd1270ae1497b295e5f7be9b9652805ee5
Author: Brijesh Bittu <[email protected]>
Date: Mon Apr 1 13:52:37 2024 +0530
Subpath export
diff --git a/pkg1/vite.config.ts b/pkg1/vite.config.ts
index b8a532f..e48eece 100644
--- a/pkg1/vite.config.ts
+++ b/pkg1/vite.config.ts
@@ -2,7 +2,7 @@ import { defineConfig } from "vite";
import react from "@vitejs/plugin-react";
import { pigment, extendTheme } from "@pigment-css/vite-plugin";
-import { theme } from "../pkg2/src";
+import { theme } from "../pkg2/src/theme";
export default defineConfig(() => ({
plugins: [
diff --git a/pkg2/src/index.ts b/pkg2/src/index.ts
index 514cada..6f6427c 100644
--- a/pkg2/src/index.ts
+++ b/pkg2/src/index.ts
@@ -1,2 +1 @@
export * as css from "./css";
-export * from "./theme";
from material-ui.
I just provided you with the simplest possible example to highlight the issue.
In the real apps, the references might happen through several files and it could start with an import of a type
(it happened to me when I tried to use Pigment in a big repo).
Please refer to the proposed solution in my previous reply.
The logic within Pigment's API is too fragile.
css
,keyframes
, etc. allalways
throw an error when invoked. Instead, they should throw an error only in development and only in browser environment.
The simple solution would be to check on the existence of
window
, when Pigment's API is invoked.
- there could be a more complex check whether Pigment is invoked in browser or during bundling
from material-ui.
Also, though this import is available @pigment-css/react/utils
and I see that you are using it in your code, but it's not documented. It's only for sharing logic internally across next/vite plugin packages.
So anything not documented should not be used in your source code and should be always be considered private and can be break at any point.
from material-ui.
@brijeshb42 in MUI X we tend to move everything that should not be imported inside an internals
folder, that way importing it feels more wrong that utils
.
from material-ui.
With that being said, I do agree that we should be crystal clear about what can be imported in a config file 👍
from material-ui.
This issue has nothing to do with internals; and it's not a question issue.
I mentioned the same thing in the previous 2 replies, here's the same summary.
The logic within Pigment's API is too fragile.
css
,keyframes
, etc. allalways
throw an error when invoked. Instead, they should throw an error only in development and only in browser environment.
The simple solution would be to check on the existence of
window
, when Pigment's API is invoked.
- there could be a more complex check whether Pigment is invoked in browser or during bundling
from material-ui.
Could you please describe the scenario were you have a file that contains css
or keyframe
and which is executed in an environment which causes an unwanted error?
from material-ui.
The scenario is outlined in the linked repo in the first comment.
The main issue is that one of the packages is exporting both styled
/css
function calls as well as a theme
object. The error arises from the fact that calling styled
/css
directly (for ex: similar to importing and calling them in the node REPL) will directly throw an error. They are supposed to be transformed through bundler plugin and compiled away.
In this case, since the package is also being imported in the vite config, there's no code transformation happening and css
is also being called which is throwing an error. It's by design. So my main proposal was that the theme (which will be used in the config) be exported through a subpath so that no css
call happens there.
It's not just about monorepo as stated. It can happen in a standalone npm package as well.
from material-ui.
Here's how vite treats a similar issue, TLDR as a bug
.
from material-ui.
Related Issues (20)
- [joy-ui][Theme] Components use radius 'sm' irrespective of their own size.
- [JOY with material-ui]
- [material-ui][Typography] Trouble adding new boolean props HOT 2
- [material-ui][Tooltip] Unable to target `MuiTooltip-tooltip` class HOT 1
- [material-ui][Slider] TypeError: color.charAt is not a function error message after upgrade @mui/material to 5.15.14 HOT 6
- [pigment-css] Invalid CSS output w/ pseudo classes in theme.vars HOT 4
- [TextField] Add an isValid state to correspond to the error state.
- [pigment-css] theme does not contain other properties except `vars` HOT 3
- [material-ui][Autocomplete] Adding start adornment on small autocompletes causes text to wrap HOT 4
- [material-ui][Switch] "Unsupported `76, 78, 100` color" error HOT 2
- Material UI components designs for Corel Vector HOT 1
- [material-ui][List] The ListItemButton component remains in the selected state even after being clicked HOT 6
- [pigment-css][vite] missing common `babel` plugin(s) & extremely slow
- [material-ui] Component import suggestion not appearing in VS Code HOT 8
- Huge gap in the line number section of the editor when on full screen. HOT 4
- [material-ui][Switch] focusElementRef is always undefined, disabling me to imperatively set focus HOT 1
- [material-ui][Menu] Error when running Jest Unit Tests: The `anchorEl` prop provided to the component is invalid.
- [material-ui] TypeError: theme.typography.pxToRem is not a function HOT 2
- [docs-infra] Support product-specific bell notifications HOT 2
- [material-ui][Tabs] Keyboard navigation does not work with unselected tabs HOT 4
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from material-ui.