feat: css support #240

Aslemammad · 2023-12-09T20:37:26Z

Follow up #171

vercel · 2023-12-09T20:37:29Z

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

Name	Status	Preview	Updated (UTC)
waku	✅ Ready (Inspect)	Visit Preview	Dec 14, 2023 1:09am

codesandbox-ci · 2023-12-09T20:52:46Z

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

Sandbox	Source
Vanilla Typescript	Configuration
React	Configuration
React TypeScript	Configuration

dai-shi

wow, this look like a great work. i'll try it later.

dai-shi · 2023-12-09T23:24:49Z

packages/waku/src/lib/rsc/worker-impl.ts

+  createServer as viteCreateServer,
+  resolveConfig as resolveViteConfig,
+  mergeConfig as mergeViteConfig,


Let's make our naming convention consistent.
Do you prefer resolveViteConfig to viteResolveConfig?

The first one is better I guess.

Fine then. We should change everything. If it's small, it can be done in this PR.

Please check all import { doSomethign as doViteSomething} from 'vite' throughout the code base. My previous code did viteDoSomething.

himself65 · 2023-12-09T23:40:29Z

packages/waku/src/lib/plugins/vite-plugin-rsc-hmr.ts

+import { createHotContext as __vite__createHotContext } from "/@vite/client"
+import.meta.hot = __vite__createHotContext(import.meta.url);
+
 if (import.meta.hot && !globalThis.__WAKU_HMR_CONFIGURED__) {
  globalThis.__WAKU_HMR_CONFIGURED__ = true;
+
  import.meta.hot.on('hot-import', (data) => import(/* @vite-ignore */ data));
+
+  import.meta.hot.on('module', (data) => {
+    const code = data.code
+    const script = document.createElement('script')
+    script.type = 'module'
+    script.text = code
+    document.head.appendChild(script)
+  });


code format inconstant. semi

You're correct, I'll fix it after Daishi reviews the code. Thank you.

please fix it.

dai-shi · 2023-12-10T12:03:03Z

When I run npm run examples:dev:12_css, there's a warning on the server console:

The CJS build of Vite's Node API is deprecated. See https://vitejs.dev/guide/troubleshooting.html#vite-cjs-node-api-deprecated for more details.

and, a warning on the bowser console:

Failed to load module script: Expected a JavaScript module script but the server responded with a MIME type of "text/html". Strict MIME type checking is enforced for module scripts per HTML spec.

Are these expected?

dai-shi

Sorry for some questions due to my limited understanding of Vite.

packages/waku/src/lib/plugins/vite-plugin-rsc-delegate.ts

packages/waku/src/lib/plugins/vite-plugin-rsc-hmr.ts

dai-shi · 2023-12-10T12:15:57Z

packages/waku/src/lib/rsc/handler-dev.ts

+    const resolvedViteConfig = await resolveViteConfig({}, 'serve');
+    const mergedViteConfig = await mergeViteConfig(
+      {
+        ...resolvedViteConfig,
+
+        plugins: resolvedViteConfig.plugins.filter(
+          (plugin) => !plugin.name.startsWith('vite:'),
+        ),
      },


What happens if we don't have this? Why do we need to remove vite: plugins?

React refresh would fail

dai-shi · 2023-12-10T12:18:58Z

packages/waku/src/lib/rsc/worker-api.ts

@@ -30,6 +31,7 @@ export type MessageReq =
 export type MessageRes =
  | { type: 'full-reload' }
  | { type: 'hot-import'; source: string }
+  | { type: 'module'; result: TransformResult }


It's ambiguous, can we call it module-import instead of module?

Why not, let me try.

dai-shi · 2023-12-10T12:19:29Z

packages/waku/src/lib/rsc/worker-api.ts

-        });
-        resolve(worker);
-      })
-      .catch((e) => reject(e));


Please don't remove this.

dai-shi · 2023-12-10T12:20:56Z

packages/waku/src/lib/rsc/worker-impl.ts

+    ...resolvedViteConfig,
+
+    plugins: resolvedViteConfig.plugins.filter(
+      (plugin) => !plugin.name.startsWith('vite:'),


Again, I wonder why we need this and if we need both in two places.

It's because we don't vite default plugins to be mentioned two times. For instance react refresh would fail because there would two instances of it.

#240 (comment)

React refresh would fail

I see. I will play with it later. Does it mean there's only one vite server that keep vite plugins? (Can't guess which from the diff, and I don't remember how many they are...)

Please extract this patch/merge logic in a separate file and reuse it. something like packages/waku/src/lib/plugins/patch-react-refresh.ts.

I don't exactly mean patching plugin. It should be a reusable logic and preferably add some comments for the intention.

Co-authored-by: Daishi Kato <dai-shi@users.noreply.github.com>

dai-shi

Looking good! Please see comments.

packages/waku/src/lib/plugins/vite-plugin-rsc-delegate.ts

dai-shi · 2023-12-13T02:03:01Z

packages/waku/src/lib/plugins/vite-plugin-rsc-hmr.ts

+import { createHotContext as __vite__createHotContext } from "/@vite/client"
+import.meta.hot = __vite__createHotContext(import.meta.url);
+
 if (import.meta.hot && !globalThis.__WAKU_HMR_CONFIGURED__) {
  globalThis.__WAKU_HMR_CONFIGURED__ = true;
+
  import.meta.hot.on('hot-import', (data) => import(/* @vite-ignore */ data));
+
+  import.meta.hot.on('module', (data) => {
+    const code = data.code
+    const script = document.createElement('script')
+    script.type = 'module'
+    script.text = code
+    document.head.appendChild(script)
+  });


please fix it.

dai-shi · 2023-12-13T02:05:54Z

packages/waku/src/lib/rsc/worker-impl.ts

+  createServer as viteCreateServer,
+  resolveConfig as resolveViteConfig,
+  mergeConfig as mergeViteConfig,


Please check all import { doSomethign as doViteSomething} from 'vite' throughout the code base. My previous code did viteDoSomething.

dai-shi · 2023-12-13T02:09:58Z

packages/waku/src/lib/rsc/worker-impl.ts

+    ...resolvedViteConfig,
+
+    plugins: resolvedViteConfig.plugins.filter(
+      (plugin) => !plugin.name.startsWith('vite:'),


#240 (comment)

React refresh would fail

I see. I will play with it later. Does it mean there's only one vite server that keep vite plugins? (Can't guess which from the diff, and I don't remember how many they are...)

Please extract this patch/merge logic in a separate file and reuse it. something like packages/waku/src/lib/plugins/patch-react-refresh.ts.

Aslemammad · 2023-12-13T19:42:41Z

I don't see the second warning, but for the first one, to be honest I don't have any idea. Can we tackle it in another PR later?

dai-shi

thanks for your work!

dai-shi · 2023-12-14T00:39:35Z

examples/12_css/vite.config.ts

+import { vanillaExtractPlugin } from '@vanilla-extract/vite-plugin';
+
+export default defineConfig({
+  root: path.dirname(url.fileURLToPath(import.meta.url)),


I think we can omit this. (Can be a separate PR, as I might be wrong, and there are other places like this.)

examples/12_css/package.json

packages/waku/src/lib/plugins/vite-plugin-rsc-hmr.ts

dai-shi · 2023-12-14T00:51:07Z

packages/waku/src/lib/utils/merge-vite-config.ts

+  mergeConfig as mergeViteConfig,
+} from 'vite';
+
+export async function mergeUserViteConfig(config: UserConfig) {


It would be nice to add a comment about this workaround is required only (?) for hmr. (Can be a separate PR.)

This reverts commit e3aa021.

This reverts commit f32f338.

* Revert "Revert "feat: css support (#240)"" This reverts commit f32f338. * possible fix * possible fix * fix 12_css package.json --------- Co-authored-by: daishi <daishi@axlight.com>

vercel bot had a problem deploying to Preview December 9, 2023 20:37 Failure

init

09a2707

Aslemammad force-pushed the fix/css-2 branch from 21c5b1b to 09a2707 Compare December 9, 2023 20:40

vercel bot had a problem deploying to Preview December 9, 2023 20:41 Failure

update

17da6ab

vercel bot deployed to Preview December 9, 2023 20:53 View deployment

dai-shi reviewed Dec 9, 2023

View reviewed changes

himself65 reviewed Dec 9, 2023

View reviewed changes

add scripts

a59e4b9

vercel bot deployed to Preview December 10, 2023 12:01 View deployment

dai-shi reviewed Dec 10, 2023

View reviewed changes

Update packages/waku/src/lib/plugins/vite-plugin-rsc-hmr.ts

6016b92

vercel bot deployed to Preview December 10, 2023 13:51 View deployment

Merge branch 'main' into fix/css-2

1f28978

vercel bot had a problem deploying to Preview December 10, 2023 13:54 Failure

missing commit

682f15e

vercel bot deployed to Preview December 10, 2023 13:56 View deployment

Merge branch 'main' into fix/css-2

1de17af

vercel bot deployed to Preview December 11, 2023 07:08 View deployment

Merge branch 'main' into fix/css-2

dc322ae

vercel bot deployed to Preview December 12, 2023 05:44 View deployment

Update packages/waku/src/lib/plugins/vite-plugin-rsc-delegate.ts

508c355

Co-authored-by: Daishi Kato <dai-shi@users.noreply.github.com>

vercel bot deployed to Preview December 12, 2023 19:36 View deployment

module to module-import

17e20d8

vercel bot deployed to Preview December 12, 2023 19:51 View deployment

add reject

bc9e17b

vercel bot deployed to Preview December 12, 2023 19:53 View deployment

Aslemammad requested review from dai-shi and himself65 December 12, 2023 19:54

dai-shi reviewed Dec 13, 2023

View reviewed changes

dai-shi linked an issue Dec 13, 2023 that may be closed by this pull request

not compatible with @vanilla-extract/css #59

Closed

dai-shi mentioned this pull request Dec 13, 2023

v1 Roadmap #24

Open

76 tasks

Aslemammad added 2 commits December 13, 2023 22:42

;

0ca5309

resolve comments

76822b2

vercel bot deployed to Preview December 13, 2023 19:38 View deployment

Merge branch 'main' into fix/css-2

9dfc698

vercel bot deployed to Preview December 14, 2023 00:38 View deployment

dai-shi approved these changes Dec 14, 2023

View reviewed changes

Apply suggestions from code review

281ae47

vercel bot had a problem deploying to Preview December 14, 2023 00:52 Failure

dai-shi added 3 commits December 14, 2023 09:54

fix pnpm lock

0eac806

Merge branch 'main' into fix/css-2

3d04f85

a small fix

7a3fdcb

vercel bot deployed to Preview December 14, 2023 01:09 View deployment

dai-shi merged commit e3aa021 into dai-shi:main Dec 14, 2023
9 checks passed

dai-shi added a commit that referenced this pull request Dec 14, 2023

Revert "feat: css support (#240)"

f32f338

This reverts commit e3aa021.

dai-shi added a commit that referenced this pull request Dec 14, 2023

Revert "Revert "feat: css support (#240)""

4acf78c

This reverts commit f32f338.

This was referenced Dec 14, 2023

Revert "Revert "feat: css support (#240)"" #259

Closed

fix: css #262

Merged

dai-shi added a commit that referenced this pull request Dec 14, 2023

fix: css (#262)

b8c35bc

* Revert "Revert "feat: css support (#240)"" This reverts commit f32f338. * possible fix * possible fix * fix 12_css package.json --------- Co-authored-by: daishi <daishi@axlight.com>

askoufis mentioned this pull request Feb 7, 2024

Build broken after vite-plugin 4.0.0 vanilla-extract-css/vanilla-extract#1315

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: css support #240

feat: css support #240

Aslemammad commented Dec 9, 2023

vercel bot commented Dec 9, 2023 •

edited

Loading

codesandbox-ci bot commented Dec 9, 2023 •

edited

Loading

dai-shi left a comment

dai-shi Dec 9, 2023

Aslemammad Dec 10, 2023

dai-shi Dec 10, 2023

dai-shi Dec 13, 2023

himself65 Dec 9, 2023

Aslemammad Dec 10, 2023

dai-shi Dec 13, 2023

dai-shi commented Dec 10, 2023

dai-shi left a comment

dai-shi Dec 10, 2023

Aslemammad Dec 12, 2023

dai-shi Dec 10, 2023

Aslemammad Dec 12, 2023

dai-shi Dec 10, 2023

dai-shi Dec 10, 2023

Aslemammad Dec 12, 2023

dai-shi Dec 13, 2023

dai-shi Dec 13, 2023

dai-shi left a comment

dai-shi Dec 13, 2023

dai-shi Dec 13, 2023

dai-shi Dec 13, 2023

Aslemammad commented Dec 13, 2023

dai-shi left a comment

dai-shi Dec 14, 2023

dai-shi Dec 14, 2023

feat: css support #240

feat: css support #240

Conversation

Aslemammad commented Dec 9, 2023

vercel bot commented Dec 9, 2023 • edited Loading

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

dai-shi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dai-shi commented Dec 10, 2023

dai-shi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dai-shi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aslemammad commented Dec 13, 2023

dai-shi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vercel bot commented Dec 9, 2023 •

edited

Loading

codesandbox-ci bot commented Dec 9, 2023 •

edited

Loading