denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
93.98k stars 5.23k forks source link

jsxImportSourceTypes not working for react #25341

Open KyleJune opened 1 week ago

KyleJune commented 1 week ago

Version: Deno 1.46.2

jsxImportSourceTypes was introduced in v1.43.0 by the following PR:

https://github.com/denoland/deno/pull/23419

It doesn't appear to be working as expected. My project isn't getting the types for react like expected with this setting.

You can reproduce the issue by cloning this example project and opening it in vs code. Then open any of the tsx files in the routes components or routes directory.

https://github.com/udibo/react-app-example

EDIT: You'll need to checkout this commit, https://github.com/udibo/react-app-example/tree/388e2bb7540daf1c88046806808f736a89467e6a because I added a react.d.ts file to workaround this issue for now.

If you try adding useState, autocomplete and import suggestions don't work right for react.

With jsxImportSourceTypes set, it won't give any suggestions from react when you type use.

image

With jsxImportSourceTypes not set, it will suggest importing the types file from my node_modules dir.

image

Then if I do that, it will get an error and the type obviously won't work.

image

If I try changing nodeModulesDir to false and repeating the tests above, the only difference is that it doesn't have any react suggestions when I type use. In that case the results are the same whether or not jsxImportSourceTypes is set.

image

If you change the import to import from react, the errors go away but it never gets the type information.

Hovering useState it just says "import useState":

image

Hovering x it has the type any:

image

The only difference I found from having jsxImportSourceTypes set is that it will stop suggesting importing from the node_modules directory if you have one. Without it, I'm not getting the errors from one of the issue it was meant to resolve: https://github.com/denoland/deno/issues/16653

KyleJune commented 1 week ago

I also tried manually adding the pragma comments, but that didn't work either.

image

image

I also tried with exactly what was specified in the runtime docs and I still had the same issue.

image

KyleJune commented 1 week ago

My deno.jsonc file has jsxRuntime as react-jsx, I just noticed that in the example from my last comment I was changing jsxRuntime to automatic. I wanted to demonstrate the issue is the same regardless of if it's in the deno.jsonc or in a pragma comment. When I changed it to react-jsx in the pragma comment, the LSP shows deno panicing reliably.

image image

Here is the output from the deno language server. I wasn't sure how to run the lsp with RUST_BACKTRACE=1 env var like it suggested.

Starting Deno language server...
  version: 1.46.2 (release, x86_64-unknown-linux-gnu)
  executable: /home/kyle/.deno/bin/deno
Connected to "Visual Studio Code" 1.92.2
Enabling import suggestions for: https://deno.land
Refreshing configuration tree...
  Resolved Deno configuration file: "file:///home/kyle/Projects/deno/react_app_example/deno.jsonc"
  Resolved lockfile: "file:///home/kyle/Projects/deno/react_app_example/deno.lock"
Server ready.

============================================================
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.46.2
Args: ["/home/kyle/.deno/bin/deno", "lsp"]

thread 'tokio-runtime-worker' panicked at /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/scoped-tls-1.0.1/src/lib.rs:168:9:
cannot access a scoped thread local variable without calling `set` first
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
[Error - 11:46:00 AM] The Deno Language Server server crashed 5 times in the last 3 minutes. The server will not be restarted. See the output for more information.
albnnc commented 1 week ago

jsxImportSourceTypes is meant to set types for jsx pragma, not any other utility functions (e.g. hooks). That being said, it works since you don't have any issues related to jsx specifically (your IDE knows what <div> is).

However, your issue is that import of "react" doesn't reference any types.

Your options are:

declare module "react" {
  // @ts-types="npm:@types/react@18"
  import * as React from "npm:react@18";
  export = React;
}

Note that you can't use "react" inside declare module "react" because of the circular refs. Using something like npm:react@18 is OK.

After finding that file Deno will pick up react-related types.

KyleJune commented 1 week ago

If I go the declaration file way, do I need to create a file with a name like react.d.ts for each module I need to specify types for or can I just put multiple declarations like that in a .d.ts file? Does deno's lsp automatically pick up all those files or do I need to add a reference to it somewhere?

I feel like deno should really be outlining these other options in their runtime docs. Currently only the deno-types in every file that needs the dependency is documented.

https://docs.deno.com/runtime/manual/node/npm_specifiers/#typescript-types

albnnc commented 1 week ago

As far as I understand, Deno picks everything in workspace root (near deno.json). You can, however, reference any .d.ts file via triple-slash directive.

And yes, you can add everything in something like global.d.ts. Note that, however, "setting" types like that is tricky since this way depends on the way particular library is structured (esm, cjs, etc.). For example, writing something like export = React is not good, but it works for react since it's written like that.

I feel like deno should really be outlining these other options in their runtime docs. Currently only the deno-types in every file that needs the dependency is documented.

Maybe, but I feel like Deno team is a little unsure about how the hell frontend should be written in Deno ecosystem. For example, JSR currently doesn't support JSX at all, Deno docs reference some dead frontend frameworks, Deno 2.0 is going to change default pragma, Node-based frameworks (not libs) don't work too well, imo.

Writing frontend with Deno is tricky indeed, but it works for sure. I can recommend these links: 1, 2, 3. To my perspective, if you can use SPA, your best way right now would be using vanilla esbuild with esbuild deno loader – it's relatively simple, pluggable and works good.

P.S. Lmao, I've just realised you're the one who tries to write [3] – I've been checking your repo a few days ago in order to find the way to get npm:react working under Deno (esm.sh/react has been working good for a long time). You're using @ts-types, and yes, crutches in react.d.ts is the only "comfortable" way I found to fix that. We should, however, blame react devs for it, and yes, Deno docs could be better.

KyleJune commented 1 week ago

P.S. Lmao, I've just realised you're the one who tries to write [3] – I've been checking your repo a few days ago in order to find the way to get npm:react working under Deno (esm.sh/react has been working good for a long time). You're using @ts-types, and yes, crutches in react.d.ts is the only "comfortable" way I found to fix that. We should, however, blame react devs for it, and yes, Deno docs could be better.

Haha yea, I'm one of the people trying to help with making it easier to do frontend work with deno. I also wrote some of the std testing utilities like bdd and mock. The last part I'm trying to figure out is getting the types working in a reasonable way. I feel like those current 3 workarounds are a bit awkward. I'll probably end up with creating a .d.ts file for each npm package I end up using that doesn't include the types. Although some users might end up with a lot of those...

I'm able to publish and use my package from jsr. I got it to work by manually adding the pragmas to each tsx file. My example works, I guess I'll just need to update it to include the react.d.ts and tell users of my package they need to include it in their projects. I also agree, esbuild seems like the best way to bundle the front end code, I have that all working along with server side rendering.

I'm currently working through writing docs/guides for my framework but will post it to showcase on discord once it's done. Might also add more examples of using react testing library and utilities to make testing easier first.

KyleJune commented 1 week ago

I ended up adding this react.d.ts file. It's similar to what @albnnc suggested. Although like mentioned in other comments, this solution is not ideal, it just seems to be the best workaround at the moment.

declare module "react" {
  // @ts-types="@types/react"
  import React from "npm:react@18";
  export = React;
}