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

Cannot use npm package @vitejs/plugin-react-swc #17213

Closed
brillout opened this issue Dec 29, 2022 · 17 comments · Fixed by #19154 or #19269
Closed

Cannot use npm package @vitejs/plugin-react-swc #17213

brillout opened this issue Dec 29, 2022 · 17 comments · Fixed by #19154 or #19269
Labels
bug Something isn't working correctly needs investigation requires further investigation before determining if it is an issue or not node compat node native extension related to the node-api (.node)

Comments

@brillout
Copy link

Reproduction: https://github.com/brillout/deno_vitejs-plugin-react-swc.

$ deno --version
deno 1.29.1 (release, x86_64-unknown-linux-gnu)
v8 10.9.194.5
typescript 4.9.4

Thanks for your wonderful work on Deno. I'd love the Vite + Deno combo (and provide a vite-plugin-ssr Deno starter).

@bartlomieju
Copy link
Member

I just tried it and can't reproduce the error you specified.

However I get this error:

11:04:04 AM [vite] Internal server error: Failed to convert napi value into rust type `Option<T>`
  Plugin: vite:react-swc
  File: /Users/ib/dev/deno_vitejs-plugin-react-swc/src/main.tsx
      at Compiler.<anonymous> (file:///Users/ib/dev/deno_vitejs-plugin-react-swc/node_modules/.deno/@[email protected]/node_modules/@swc/core/index.js:216:33)
      at Generator.next (<anonymous>)
      at file:///Users/ib/dev/deno_vitejs-plugin-react-swc/node_modules/.deno/@[email protected]/node_modules/@swc/core/index.js:34:71
      at new Promise (<anonymous>)
      at __awaiter (file:///Users/ib/dev/deno_vitejs-plugin-react-swc/node_modules/.deno/@[email protected]/node_modules/@swc/core/index.js:30:12)
      at Compiler.transform (file:///Users/ib/dev/deno_vitejs-plugin-react-swc/node_modules/.deno/@[email protected]/node_modules/@swc/core/index.js:202:16)
      at transform (file:///Users/ib/dev/deno_vitejs-plugin-react-swc/node_modules/.deno/@[email protected]/node_modules/@swc/core/index.js:344:21)
      at TransformContext.transform (file:///Users/ib/dev/deno_vitejs-plugin-react-swc/node_modules/.deno/@[email protected]/node_modules/@vitejs/plugin-react-swc/index.mjs:42:24)
      at Object.transform (file:///Users/ib/dev/deno_vitejs-plugin-react-swc/node_modules/.deno/[email protected]/node_modules/vite/dist/node/chunks/dep-0bae2027.js:41480:44)
      at async loadAndTransform (file:///Users/ib/dev/deno_vitejs-plugin-react-swc/node_modules/.deno/[email protected]/node_modules/vite/dist/node/chunks/dep-0bae2027.js:39287:29)

@bartlomieju bartlomieju added needs investigation requires further investigation before determining if it is an issue or not node compat labels Dec 29, 2022
@bartlomieju
Copy link
Member

@brillout tangentially, @itsdouges and I are working on Vite resolver for Deno at https://github.com/itsdouges/vite_plugin_deno_resolve

@brillout brillout changed the title Cannot install npm package @vitejs/plugin-react-swc Cannot use npm package @vitejs/plugin-react-swc Dec 29, 2022
@brillout
Copy link
Author

Hm, indeed, I can't reproduce anymore as well. I now get the same error than yours. I've updated the ticket & reproduction.

@brillout tangentially, @itsdouges and I are working on Vite resolver for Deno at https://github.com/itsdouges/vite_plugin_deno_resolve

That's very nice...

The, by far, number 1 problem vite-plugin-ssr users are getting is ESM/CJS problems (e.g. https://vite-plugin-ssr.com/invalid-esm and that's just a fraction of it). Node.js is deeply broken here and there isn't much vite(-plugin-ssr) can do about it. There is a big opportunity for Deno to make things right.

FYI I'm working on a React framework and I'd love to make it use Deno by default. If we systematically fix all problems, I think we can build something truly magical. (I systematically fix all vite-plugin-ssr bugs and, to this day, it has zero known bugs.)

@bartlomieju
Copy link
Member

Hm, indeed, I can't reproduce anymore as well. I now get the same error than yours. I've updated the ticket & reproduction.

@brillout tangentially, @itsdouges and I are working on Vite resolver for Deno at https://github.com/itsdouges/vite_plugin_deno_resolve

That's very nice...

The, by far, number 1 problem vite-plugin-ssr users are getting is ESM/CJS problems (e.g. https://vite-plugin-ssr.com/invalid-esm and that's just a fraction of it). Node.js is deeply broken here and there isn't much vite(-plugin-ssr) can do about it. There is a big opportunity for Deno to make things right.

How would you approach it? Internally in Deno we translate CJS code to ESM on the fly, but it's internal functionality that's currently not exposed in any way.

FYI I'm working on a React framework and I'd love to make it use Deno by default. If we systematically fix all problems, I think we can build something truly magical. (I systematically fix all vite-plugin-ssr bugs and, to this day, it has zero known bugs.)

Sounds good, let me know if there's anything concrete we can help with here.

@brillout
Copy link
Author

How would you approach it? Internally in Deno we translate CJS code to ESM on the fly, but it's internal functionality that's currently not exposed in any way.

I'd make Deno permissive.

For example, a widespread problem is npm packages having invalid ESM imports, e.g. import './foo' instead of import './foo.js', which makes Node.js choke. Another example is packages that ship ESM but don't have type: "module" in their package.json. Deno should be able to execute npm packages that ship invalid code.

I'd make Deno permissive only for npm packages, while being more strict for user land and Deno packages. In order to foster a Deno ecosystem that is correct.

Sounds good, let me know if there's anything concrete we can help with here.

I think the number one goal for now is to fix the many problems users encounter because of Node.js.

Together with Vite we have a unique opportunity to own the entire stack while fostering a diverse ecosystem (in contrast to other companies' tendency to control everything and being ecosystem-unfriendly), enabling folks to inovate like Svelte, Solid, Hydrogen/Preact, etc. Our "only" job being to offer basic tools that work very well. Vite is hitting a limit because there is no communication between the Vite team and the Node.js team (I'm not sure if the Node.js team is intersted in that to begin with).

On my side, let me first show you a prototype of that React framework "Base". I think you'll like what you see.

(Base is only a thin wrapper on top of vite-plugin-ssr, which means that improvements also benefits the entire Vite ecosystem.)

Ideally I'll ship Base with Deno right away. We can then systematically polish Deno to fix problems, most notably the many ESM/CJS issues users are having today. FYI another pet peeve of mine is that errors can be swallowed. That's a no-go, exceptions should always emit a stack trace. (Although I'm not sure whether it's a V8 problem that Deno cannot work around.)

I've always like Deno, because Deno cares, truly cares.

@ghost
Copy link

ghost commented Apr 8, 2023

I had the same problem when working React + Vite + Typescript + SWC templates.

deno run --allow-read --allow-write --allow-env npm:create-vite-extra@latest

@ghost
Copy link

ghost commented Apr 8, 2023

@bartlomieju
This plugin didn't solve the problem, does this plugin still work?

@scarf005
Copy link
Contributor

scarf005 commented May 5, 2023

asciicast

@bartlomieju using create-vite-extra with typescript-swc still emits error due to napi and Option mismatch. when i change react plugin to import react from 'npm:@vitejs/plugin-react@^4.0.0', deno crashes.

Panic backtrace
Deno has panicked. This is a bug in Deno. Please report this
at https://github.com/denoland/deno/issues/new.
If you can reliably reproduce this panic, include the
reproduction steps and re-run with the RUST_BACKTRACE=1 env
var set and include the backtrace in your report.

Platform: linux x86_64
Version: 1.33.2
Args: ["/home/scarf/.deno/bin/deno", "run", "-A", "--unstable", "--node-modules-dir", "npm:vite"]

thread 'main' panicked at 'Attemped to access invalid request 3 (0 in total available)', ext/http/http_next.rs:195:1
stack backtrace:
   0: rust_begin_unwind
             at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/panicking.rs:575:5
   1: core::panicking::panic_fmt
             at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/core/src/panicking.rs:64:14
   2: deno_http::http_next::op_set_response_headers::v8_fn_ptr
   3: _ZN2v88internal12_GLOBAL__N_119HandleApiCallHelperILb0EEENS0_11MaybeHandleINS0_6ObjectEEEPNS0_7IsolateENS0_6HandleINS0_10HeapObjectEEENS8_INS0_20FunctionTemplateInfoEEENS8_IS4_EEPmi
             at ./../../../../v8/src/builtins/builtins-api.cc:113:36
   4: _ZN2v88internal21Builtin_HandleApiCallEiPmPNS0_7IsolateE
             at ./../../../../v8/src/builtins/builtins-api.cc:135:1
   5: Builtins_CEntry_Return1_ArgvOnStack_BuiltinExit
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

@mmastrac
Copy link
Contributor

This stack trace looks like it will be fixed by #19154

mmastrac added a commit that referenced this issue May 16, 2023
Fixes for various `Attemped to access invalid request` bugs (#19058,
#15427, #17213).

We did not wait for both a drop event and a completion event before
removing items from the slab table. This ensures that we do so.

In addition, the slab methods are refactored out into `slab.rs` for
maintainability.
levex pushed a commit that referenced this issue May 18, 2023
Fixes for various `Attemped to access invalid request` bugs (#19058,
#15427, #17213).

We did not wait for both a drop event and a completion event before
removing items from the slab table. This ensures that we do so.

In addition, the slab methods are refactored out into `slab.rs` for
maintainability.
@danopia
Copy link
Contributor

danopia commented May 24, 2023

Hi, I'm importing npm:@swc/core directly (no vite) and I encountered the "napi" stacktrack which was posted early up in this thread. Here's my version:

error: Uncaught Error: Failed to convert napi value into rust type `Option<T>`
    at Compiler.<anonymous> (file:///home/dan/.cache/deno/npm/registry.npmjs.org/@swc/core/1.3.59/index.js:216:33)
    at Generator.next (<anonymous>)
    at file:///home/dan/.cache/deno/npm/registry.npmjs.org/@swc/core/1.3.59/index.js:34:71
    at new Promise (<anonymous>)
    at __awaiter (file:///home/dan/.cache/deno/npm/registry.npmjs.org/@swc/core/1.3.59/index.js:30:12)
    at Compiler.transform (file:///home/dan/.cache/deno/npm/registry.npmjs.org/@swc/core/1.3.59/index.js:202:16)
    at Object.transform (file:///home/dan/.cache/deno/npm/registry.npmjs.org/@swc/core/1.3.59/index.js:344:21)
    at AppBuilder.piecemealSourceFromFiles (file:///home/dan/Code/danopia/dist-app-bundles/sdk/app-builder.ts:56:8)

I checked the fix PR above #19154 and it doesn't seem related to the napi mismatch crash. Did something else address this stack trace? I've tried the latest deno canary (e56695d) and it still has the napi crash.

@bartlomieju bartlomieju reopened this May 24, 2023
@bartlomieju
Copy link
Member

I think it was closed by mistake. @danopia could you provide full reproduction so I can try it myself too? I can probably fix this error for the next patch release in a few days.

@danopia
Copy link
Contributor

danopia commented May 24, 2023

Sure, here's a minimal repro of @swc/core crashing:

#!/usr/bin/env -S deno run --allow-env --allow-read --allow-run --allow-ffi
import swc from "npm:@swc/core";
console.log(await swc.transform("export {}"));
> ./repro.ts 
error: Uncaught Error: Failed to convert napi value into rust type `Option<T>`
    at Compiler.<anonymous> (file:///home/dan/.cache/deno/npm/registry.npmjs.org/@swc/core/1.3.59/index.js:216:33)
    at Generator.next (<anonymous>)
    at file:///home/dan/.cache/deno/npm/registry.npmjs.org/@swc/core/1.3.59/index.js:34:71
    at new Promise (<anonymous>)
    at __awaiter (file:///home/dan/.cache/deno/npm/registry.npmjs.org/@swc/core/1.3.59/index.js:30:12)
    at Compiler.transform (file:///home/dan/.cache/deno/npm/registry.npmjs.org/@swc/core/1.3.59/index.js:202:16)
    at Object.transform (file:///home/dan/.cache/deno/npm/registry.npmjs.org/@swc/core/1.3.59/index.js:344:21)
    at file:///home/dan/Code/danopia/repro.ts:3:33

@bartlomieju
Copy link
Member

Thanks, that's very useful. I traced the problem to the call to napi_typeof, which somehow passes an empty pointer as an argument, which is explicitly guarded against. Since it works in Node I assume it's a problem in another napi_ symbol we have that doesn't return a proper value.

@bartlomieju bartlomieju added bug Something isn't working correctly node native extension related to the node-api (.node) labels May 24, 2023
@bartlomieju
Copy link
Member

We've managed to track down the bug here. Will try to fix it tonight.

@liudonghua123
Copy link

I have the same problems when I tried deno deploy and react-swc template.

image

@bartlomieju
Copy link
Member

This fix hasn't been released yet. I will be later tonight/tomorrow.

@danopia
Copy link
Contributor

danopia commented May 29, 2023

Thank you for personally addressing this issue @bartlomieju ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly needs investigation requires further investigation before determining if it is an issue or not node compat node native extension related to the node-api (.node)
Projects
None yet
6 participants