Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: add @stylexjs/stylex example #286

Merged
merged 16 commits into from
Dec 21, 2023
Merged

feat: add @stylexjs/stylex example #286

merged 16 commits into from
Dec 21, 2023

Conversation

himself65
Copy link
Sponsor Contributor

No description provided.

@vercel Vercel
Copy link

vercel bot commented Dec 17, 2023 •
edited
Loading

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

Name Status Preview Updated (UTC)
waku ✅ Ready (Inspect) Visit Preview Dec 21, 2023 0:13am

@himself65 himself65 mentioned this pull request Dec 17, 2023
Closed
@codesandbox-ci CodeSandbox CI
Copy link

codesandbox-ci bot commented Dec 17, 2023 •
edited
Loading

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

Sandbox Source
Vanilla Typescript Configuration
React Configuration
React TypeScript Configuration

himself65
himself65 commented Dec 17, 2023
View reviewed changes
Copy link
Sponsor Contributor Author

@himself65 himself65 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First. stylex doesn't support esm module, so I use vite cjs plugin to transform it to esm. But I think in dev mode, server inline the code twice so make the error happened

himself65
himself65 commented Dec 17, 2023
View reviewed changes
Copy link
Sponsor Contributor Author

@himself65 himself65 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First. stylex doesn't support esm module, so I use vite cjs plugin to transform it to esm. But I think in dev mode, server inline the code twice so make the error happened

@dai-shi
Copy link
Owner

dai-shi commented Dec 17, 2023

stylex doesn't support esm module

So, is it essentially the same as #110?

@himself65
Copy link
Sponsor Contributor Author

himself65 commented Dec 18, 2023 •
edited
Loading

stylex doesn't support esm module

So, is it essentially the same as #110?

CJS not supported is the vite issue. They suggest using vite-plugin-commonjs. However, doesn't work in waku dev mode.

himself65
himself65 commented Dec 18, 2023
View reviewed changes
Copy link
Sponsor Contributor Author

@himself65 himself65 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I investigated a while and I believe is plugin stylex bug with SSR

@himself65 himself65 mentioned this pull request Dec 18, 2023
Open
@HorusGoul
Copy link

The plugin mostly works (https://github.com/HorusGoul/waku-stylex-demo), but I encountered an issue that prevented me from installing a Waku demo on the mono repo of the plugin to add tests to it.

I believe it has to do with having multiple react versions in the same mono repo, for some reason, Waku is trying to pick the one from the top-level node_modules, see this PR: HorusGoul/vite-plugin-stylex#22

@himself65 himself65 changed the title feat: add example stylex feat: add @stylexjs/stylex example Dec 18, 2023
@himself65 himself65 marked this pull request as ready for review December 18, 2023 19:24
@himself65
Copy link
Sponsor Contributor Author

I believe it has to do with having multiple react versions in the same mono repo, for some reason, Waku is trying to pick the one from the top-level node_modules, see this PR: HorusGoul/vite-plugin-stylex#22

/cc @dai-shi any idea? I think this is package manager behavior

@dai-shi
Copy link
Owner

dai-shi commented Dec 18, 2023

I'm not quite following. So, is it like this PR is working for @himself65 without dual package issue, but @HorusGoul has the dual package issue?

@dai-shi
Copy link
Owner

dai-shi commented Dec 19, 2023

Ok, I investigated a while and I believe is plugin stylex bug with SSR

What does it mean? It feels like this example has a limitation, or no?

@himself65
Copy link
Sponsor Contributor Author

Ok, I investigated a while and I believe is plugin stylex bug with SSR

What does it mean? It feels like this example has a limitation, or no?

Just we have to add @stylex stylesheet; in one CSS stylesheet to make things work. Not a limitation

@himself65
Copy link
Sponsor Contributor Author

I'm not quite following. So, is it like this PR is working for @himself65 without dual package issue, but @HorusGoul has the dual package issue?

No. dual package issue happens on his repo. Not waku

dai-shi
dai-shi reviewed Dec 21, 2023
View reviewed changes
examples/12_css/package.json Outdated Show resolved Hide resolved
package.json
Comment on lines +79 to +82
"pnpm": {
"overrides": {
"vite": "5.0.9"
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even with this?

Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, some upstream package use different version of vite

@dai-shi dai-shi merged commit 44fe5cb into dai-shi:main Dec 21, 2023
9 checks passed
@himself65 himself65 deleted the himself65/stylex branch December 21, 2023 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dai-shi dai-shi dai-shi left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@himself65 @dai-shi @HorusGoul