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.

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

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

Closed
1 task done
bronifty opened this issue Nov 3, 2023 · 5 comments
Closed
1 task done

Comments

@bronifty
Copy link

bronifty commented Nov 3, 2023

What version of Remix are you using?

2.2.0

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

  • Yes

Steps to Reproduce

use remix as a vite plugin, build an app with a tsx route that consumes mdx via the @mdx-js/rollup plugin and build. head to the mdx consumer route and it breaks with the error message
TypeError: t.jsxDEV is not a function
at o (http://localhost:3000/build/assets/test-1a250893.js:9:223)
at l (http://localhost:3000/build/assets/test-1a250893.js:10:578)
at io (http://localhost:3000/build/assets/entry.client-1dd82fdc.js:68:19477)
at ja (http://localhost:3000/build/assets/entry.client-1dd82fdc.js:70:43691)
at Da (http://localhost:3000/build/assets/entry.client-1dd82fdc.js:70:39496)
at id (http://localhost:3000/build/assets/entry.client-1dd82fdc.js:70:39427)
at $r (http://localhost:3000/build/assets/entry.client-1dd82fdc.js:70:39287)
at Ni (http://localhost:3000/build/assets/entry.client-1dd82fdc.js:70:35709)
at Ta (http://localhost:3000/build/assets/entry.client-1dd82fdc.js:70:34665)
at E (http://localhost:3000/build/assets/entry.client-1dd82fdc.js:55:1562)

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
Copy link
Contributor

hi-ogawa commented Nov 4, 2023

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
$ 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/@[email protected][email protected][email protected][email protected]/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 #7911)

plugins: [
...(viteUserConfig.plugins ?? [])

@bronifty
Copy link
Author

bronifty commented Nov 5, 2023

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

@hi-ogawa
Copy link
Contributor

hi-ogawa commented Nov 5, 2023

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

@bronifty
Copy link
Author

bronifty commented Nov 6, 2023

no problem thank you!

@markdalgleish
Copy link
Member

Fixed by #7911.

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

No branches or pull requests

4 participants