fix: add new build for styleless

[Jokcy]

What kind of change does this pull request introduce?

We used try to remove the typeof process condition for default styles, but it bring a new issue for vite and a new PR #795 add the condition back.

We need to understand that this kind of assert will always be undefinde for most of bundlers, vite/webpack etc, because there is no process in browser and all of them chose to directly replace process.env.xxx to the specify value, this means:

const a = process.env.xxx // source code

// after compile

const a = true // the value you specified on config

So without remove the typeof process condition the disable default style feature will never work on production (for some reason most of bundler actually provide a process object on dev mode).

It's not recommeded for client module to rely on procee.env.xxx env variables because it's not standard. Different bundle tools handle env with different ways, for example in vite the default supported way to get env is through import.meta.env.xxx, so there is no process.env unless you config it at vite.config.js.

What is the current behavior?

Disable default style will never work on production build.

What is the new behavior?

Add a new build to remove the default style, the remove action happend at build time which we can control. And we can do more with this to remove all the css({...}) code in this build to even reduce the final bundle size.

What steps did you take to test this? This is required before we can merge, make sure to test the flow you've updated.

Build with stitches

Build without stitches

Checklist

• Documentation;
• Storybook (if applicable);
• Tests;
• Ready to be merged;

[vercel]

@Jokcy is attempting to deploy a commit to the CodeSandbox Team on Vercel.

A member of the Team first needs to authorize it.

[codesandbox-ci]

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 e0e8739:

Sandbox	Source
Sandpack	Configuration
icy-bush-g4d5ww	Configuration

[danilowoz]

Hey, @Jokcy! I loved this solution!

I pushed another version that relies on a new folder, which contains all components without styling:

import { Sandpack } from "@codesandbox/sandpack-react/unstyled";

What do you think?

[Jokcy]

@danilowoz That's great! I did want to do the same thing but I'm not sure if you will agree this solution so I just implement it in minimal effect. Great job anyway.

I also want to remove the default css code for the unstyled bundle, for example:

// in Stack.tsx

export const stackClassName = css({
  display: "flex",
  flexDirection: "column",
  width: "100%",
  position: "relative",
  backgroundColor: "$colors$surface1",
  gap: 1, // border between components

  [`&:has(.${THEME_PREFIX}-stack)`]: {
    backgroundColor: "$colors$surface2",
  },
});

These code is useless for unstyled bundle, I want to figure out a easy way to remove all these code without a lot of change of our codebase.

Do you want this being include in this PR or another PR should be created?

[danilowoz]

Thanks! Yeah, let's try to remove these styles on this PR, feel free to give it a try.

[Jokcy]

@danilowoz I added an rollup plugin to remove the css code for unstyled bundle, please review it and give some feedback. It did reduce the size after add this plugin:

I also added the filesize plugin to help show file size.

[danilowoz]

THIS IS GREAT! 🚀

I was wondering if, in the future, we can extract the CSS to static files. What do you think?

[Jokcy]

I was wondering if, in the future, we can extract the CSS to static files. What do you think?

🤔 You mean like pre compile stitches to css files?

[jeetiss]

You mean like pre compile stitches to css files?

sounds like https://vanilla-extract.style/

it has plugins for esbuild and rollup to extract styles, but i'm not sure that it will fit sandpack well.

I struggle with current styles a lot and it should be much easier to work with good old CSS

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated
sandpack-docs	✅ Ready (Inspect)	Visit Preview	💬 Add your feedback	Mar 14, 2023 at 10:46AM (UTC)

[Jokcy]

@danilowoz If we generate static css from stitches, I guess it may break the theme props usage?

[danilowoz]

yeah, it's not an easy issue to solve.

BTW, can you update this branch with main?

[Jokcy]

BTW, can you update this branch with main?

You mean rebase main to this branch?

[Jokcy]

@danilowoz done.

[danilowoz]

Hey @Jokcy. It seems that this PR introduces a regression on the Console component. I'm getting the following error only on the documentation (dev-mode works fine), which made me conclude that the bundler has been broken.

I spent quite some time trying to figure out what was going on, but with no success. Maybe you have some idea.

[Jokcy]

@danilowoz Got it, will check later. Do we have some unit/e2e tests to help us more confident about the changes? The unstyled bundle should pass all the cases.

[danilowoz]

No, we don't. But I'm not if a unit test could help in this case because it looks a like dependency or a bundler issue

[Jokcy]

@danilowoz The reason is import Ansi from "ansi-to-react"; result to {__esModule: true, default: ƒ} instead of just get the default Ansi component. It's kind of weird...

[Jokcy]

@danilowoz Find the reason, it's because we output our es bundle to mjs, which seems being handled differently in typescript, change file to index.js and "import": "./dist/index.js" in packagejson fix this issue. Don't know why though, need to do some research.

From: https://www.typescriptlang.org/docs/handbook/esm-node.html

imports might resolve differently from dependencies in node_modules

Seems it did have some difference for typescript to handle import in an esm file.

The easier way is to output filename like index.esm.js.

[jeetiss]

I fulgurate out and this is how webpack works. it doesn't apply export default interop to mjs files, breaking the "ansi-to-react" dependency. this is a modern module resolution algorithm from the nodejs and it is enabled only for mjs files

The easier way is to output filename like index.esm.js.

so a name with js extension will solve the problem.

here is repo with minimal reproduction https://github.com/jeetiss/mjs-commonjs-default

[danilowoz]

Thank you @Jokcy! Thank you @jeetiss!

Let's get this merged