Skip to content

[core] Errors thrown in non-browser environment (unsafe when referenced) #8

Open

Description

@o-alexandrov

Steps to reproduce

Link to live example

Steps:

  1. Clone repo
  2. Install deps npm install (from repo's root)
  3. Run npm run start (from pkg1)
  4. Observe error in your CLI
Error: @pigment-css/react: You were trying to call "css" function without configuring your bundler
  1. Comment out theme in vite.config.ts
  2. Restart npm run start (it will run without issues)

Current behavior

pigment-css-vite-plugin doesn't work when a theme is connected directly (imports a value) or imports a type from one of the files that is importing from a file with @pigment-css/react's css.

Expected behavior

Theme should be shareable and references shouldn't break the pigment-css-vite-plugin unexpectedly.

Context

Btw, the bug exists in both pigment's 0.0.4 and the latest state of mui's next branch (checked a couple of hours ago)

Your environment

npx @mui/envinfo

  System:
    OS: macOS 14.4
  Binaries:
    Node: 21.7.1 - /opt/homebrew/bin/node
    npm: 10.5.0 - /opt/homebrew/bin/npm
    pnpm: 8.15.5 - /opt/homebrew/bin/pnpm
  Browsers:
    Chrome: 123.0.6312.87
    Edge: 112.0.1722.39
    Safari: 17.4
  npmPackages:
    @pigment-css/react: 0.0.4
    @pigment-css/vite-plugin: 0.0.4
    @emotion/react:  11.11.4 
    @emotion/styled:  11.11.5 
    @mui/private-theming:  5.15.14 
    @mui/styled-engine:  6.0.0-alpha.0 
    @mui/system:  6.0.0-alpha.0 
    @mui/types:  7.2.14 
    @mui/utils:  5.15.14 
    @types/react: 18.2.73 => 18.2.73 
    react: ^18.2.0 => 18.2.0 
    react-dom: ^18.2.0 => 18.2.0 
    typescript: 5.4.3 => 5.4.3 

Search keywords: pigment vite monorepo

Activity

  1. self-assigned this
    on Apr 1, 2024
  2. changed the title [-][pigment-css] vite plugin. not working when used in a monorepo (w/ shareable theme)[/-] [+][pigment-css][vite] Plugin not working when used in a monorepo (w/ shareable theme)[/+] on Apr 1, 2024
  3. brijeshb42 commented on Apr 1, 2024

    @brijeshb42
    Contributor

    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.

  4. added
    support: questionCommunity support but can be turned into an improvement.
    and removed on Apr 1, 2024
  5. o-alexandrov commented on Apr 1, 2024

    @o-alexandrov
    Author

    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 even a 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
  6. brijeshb42 commented on Apr 1, 2024

    @brijeshb42
    Contributor

    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 <brijeshb42@gmail.com>
    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";
  7. o-alexandrov commented on Apr 1, 2024

    @o-alexandrov
    Author

    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. all always 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
  8. brijeshb42 commented on Apr 1, 2024

    @brijeshb42
    Contributor

    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.

  9. flaviendelangle commented on Apr 2, 2024

    @flaviendelangle
    Member

    @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.

  10. brijeshb42 commented on Apr 2, 2024

    @brijeshb42
    Contributor
  11. flaviendelangle commented on Apr 2, 2024

    @flaviendelangle
    Member

    With that being said, I do agree that we should be crystal clear about what can be imported in a config file 👍

  12. brijeshb42 commented on Apr 2, 2024

    @brijeshb42
    Contributor

    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.

  13. o-alexandrov commented on Apr 2, 2024

    @o-alexandrov
    Author

    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. all always 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
  14. flaviendelangle commented on Apr 3, 2024

    @flaviendelangle
    Member

    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?

  15. brijeshb42 commented on Apr 3, 2024

    @brijeshb42
    Contributor

    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.

  16. o-alexandrov commented on Apr 3, 2024

    @o-alexandrov
    Author

    Here's how vite treats a similar issue, TLDR as a bug.

  17. changed the title [-][pigment-css][vite] Plugin not working when used in a monorepo (w/ shareable theme)[/-] [+][pigment-css][react] throws errors in non-browser environment (i.e. unsafe when referenced)[/+] on Apr 4, 2024
  18. changed the title [-][pigment-css][react] throws errors in non-browser environment (i.e. unsafe when referenced)[/-] [+][core] Errors thrown in non-browser environment (unsafe when referenced)[/+] on Apr 16, 2024
  19. o-alexandrov commented on Feb 19, 2025

    @o-alexandrov
    Author

    A temporary workaround is to set:

    global.__wyw_dynamic_import = 1

    This makes css and other PigmentCSS API non-breaking (non-throwing).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

support: questionCommunity support but can be turned into an improvement.

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions