Running Consumer Route of MDX Fails on Vite Plugin version of Remix

bronifty commented 1 year ago

What version of Remix are you using?

2.2.0

Are all your remix dependencies & dev-dependencies using the same version?

[X] Yes

Steps to Reproduce

Expected Behavior

i expect it to not break

Actual Behavior

it breaks. here's the repo to repro: https://github.com/bronifty/migrate-remix-to-vite.git

hi-ogawa commented 1 year ago

It looks like mdx plugin is transpiling JSX into jsxDEV(...) instead of jsx(...) even for build command. I tested your reproduction and setting mdx plugin's development option explicitly seems to solve the issue:

export default defineConfig((env) => ({
  plugins: [
   ...
    mdx({
      development: env.command === "serve",  /// <==== switch based on `vite dev` or `vite build`
      remarkPlugins: [
        remarkFrontmatter,
        remarkMdxFrontmatter,
      ],
    })
  ],
}));

If this is not handled by mdx plugin itself, maybe it's worth mentioning in remix doc https://remix.run/docs/en/main/future/vite#add-mdx-plugin

EDIT: I realized that mdx plugin is a rollup plugin and thus not dedicated as a vite plugin, so it seems configuring development option is vite user's responsibility https://mdxjs.com/packages/rollup/#options.

EDIT2: Investigating further, it looks like rollup plugin actually infers development option based on mode https://github.com/mdx-js/mdx/blob/efff74e48aec9286c62185bf04f7b8aa72e07bd6/packages/rollup/lib/index.js#L79, so I would feel user-side explicit configuration is not necessarily for normal case. The reason why this is not working might be because of some inconsistency with viteChildCompiler used by remix...?

EDIT3: Confirming the above assumption by debugging with this plugin:

function debugPlugin(): Plugin {
  let count = 0;

  return {
    name: "debug",

    config(_config, env) {
      console.log("[debug:config]", ++count, env);
    },

    buildEnd() {
      console.log("[debug:buildEnd]", count);
    },
  }
}

This will show following logs:

pnpm dev

``` $ pnpm dev > remix-vite-dup-plugin@ dev /home/hiroshi/code/tmp/remix-vite-plugin-twice > vite dev [debug:config] 1 { mode: 'development', command: 'serve', ssrBuild: false } [debug:config] 2 { mode: 'development', command: 'serve', ssrBuild: false } ```

pnpm build

```sh $ pnpm build > remix-vite-dup-plugin@ build /home/hiroshi/code/tmp/remix-vite-plugin-twice > vite build && vite build --ssr [debug:config] 1 { mode: 'production', command: 'build', ssrBuild: false } //// <======= debug log [debug:config] 2 { mode: 'development', command: 'serve', ssrBuild: false } //// <============ vite v4.5.0 building for production... ⚠️ Remix support for Vite is unstable and not recommended for production transforming (38) node_modules/.pnpm/@remix-run+react@2.2.0_react-dom@18.2.0_react@18.2.0_typescript@5.2.2/node_modules/@remix-run/react/dist/esm/routeModules.js[debug:buildEnd] 2 ✓ 50 modules transformed. [debug:buildEnd] 2 //// <============ (!) The public directory feature may not work correctly. outDir /home/hiroshi/code/tmp/remix-vite-plugin-twice/public/build and publicDir /home/hiroshi/code/tmp/remix-vite-plugin-twice/public are not separate folders. public/build/manifest.json 1.11 kB │ gzip: 0.32 kB public/build/assets/_index-891f6d53.js 0.76 kB │ gzip: 0.38 kB public/build/assets/root-746b886e.js 1.39 kB │ gzip: 0.82 kB public/build/assets/jsx-runtime-26afeca0.js 8.09 kB │ gzip: 3.04 kB public/build/assets/components-a34628f9.js 75.50 kB │ gzip: 24.93 kB public/build/assets/entry.client-633fb105.js 143.55 kB │ gzip: 46.57 kB ✓ built in 1.07s [debug:config] 1 { mode: 'production', command: 'build', ssrBuild: true } //// <============ [debug:config] 2 { mode: 'development', command: 'serve', ssrBuild: true } //// <============ vite v4.5.0 building SSR bundle for production... ⚠️ Remix support for Vite is unstable and not recommended for production transforming (1) virtual:server-entry[debug:buildEnd] 2 //// <============ ✓ 6 modules transformed. [debug:buildEnd] 2 build/index.js 7.35 kB ✓ built in 137ms ```

I think simply copying "plugin objects" to child vite server can lead to some plugin hooks called in an unexpected way since it's common to save some state in "plugin factory" closure. (Alternative would be reading vite.config.ts again for child compiler so that "plugin instances" are independent...?)

At least, it probably makes sense to pass same mode to child compiler so that it won't confuse plugins like mdx rollup plugin which makes use of mode. (EDIT: created a PR https://github.com/remix-run/remix/pull/7911)

https://github.com/remix-run/remix/blob/3874120b7b1f93cb4830a845414af430f577fd81/packages/remix-dev/vite/plugin.ts#L494-L495

bronifty commented 1 year ago

it was tough following this thread but I checked the file you changed in the PR and it works thanks

hi-ogawa commented 1 year ago

Sorry about my comment derailing a bit too much... Thanks for testing and providing the reproduction!

bronifty commented 1 year ago

no problem thank you!

markdalgleish commented 1 year ago

Fixed by #7911.

remix-run / remix