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

v0.35.0 TSX file can't be compiled #4197

Closed
keroxp opened this issue Feb 29, 2020 · 21 comments · Fixed by #4376
Closed

v0.35.0 TSX file can't be compiled #4197

keroxp opened this issue Feb 29, 2020 · 21 comments · Fixed by #4376

Comments

@keroxp
Copy link
Contributor

keroxp commented Feb 29, 2020

index.tsx

import React from "https://dev.jspm.io/react"
export default () => <html>deno</html>;
error TS7026: JSX element implicitly has type 'any' because no interface 'JSX.IntrinsicElements' exists.

► file:///Users/keroxp/src/deno/index.tsx:11:22

11 export default () => <html>deno</html>;

I know recently strict option enabled by default for deno compiler but I couldn't right way to define correct JSX namespace for external resources.

@tamoore
Copy link

tamoore commented Mar 1, 2020

If you change the file ext to jsx it works. Seems to only be an issue with tsx files. Haven't had time to look deeper.

@keroxp
Copy link
Contributor Author

keroxp commented Mar 1, 2020

@tamoore Thank you. But I found the way.

By official guide, triple-slash reference to type definition is needed for React or other type-aware frameworks. Indeed adding it on top of .tsx file resolved the compile error.

/// <reference path="path/to/react.d.ts" />
import React from "https://dev.jspm.io/react"
export default () => <html>deno</html>;

I'm using patched react d.ts (to ESM) for my project with @deno-types but this doesn't work by alone. I'm confused why they are needed independently.

// @deno-types="../../../../types/react/index.d.ts"
export * from "https://dev.jspm.io/react@16.10.2/index.js";

If indirect cause of the problem is --no-implicit-any in #3899 , excluding it from compiler default is also the way of resolution. It may be a subject for other discussion.

@keroxp
Copy link
Contributor Author

keroxp commented Mar 1, 2020

This also works and seems to be the correct way.

import React from "https://dev.jspm.io/react"
import { FC } from "path/to/react.d.ts"
export default (() => <html>deno</html>) as FC;

@kitsonk
Copy link
Contributor

kitsonk commented Mar 3, 2020

It is a combination of things, #4040 which stopped analysing JavaScript imports as well as forcing strict mode.

This should work:

// @deno-types="../../../../types/react/index.d.ts"
export * from "https://dev.jspm.io/react@16.10.2/index.js";

The path references should also work, though we don't have tests for their resolution, so they might not work the way the work on tsc, and ideally you wouldn't use them under Deno.

I will look into why we broke that, as that certainly wasn't intended.

What is the ultimate use case here? As the runtime compiler APIs are in place and if you are trying to generate something that you run in a browser, that might be better suited. I still have work to do before landing it in standard, but take a look at this: https://github.com/kitsonk/deno/blob/std-react/std/build/react.ts

The following code works, with no "magical" things:

const [diagnostics, output] = await Deno.compile(
  "./std/build/examples/react-index.jsx",
  undefined,
  {
    allowJs: true,
    checkJs: true,
    lib: ["dom", "es2018"],
    sourceMap: false,
    target: "es2018",
    types: ["./std/build/types/react.d.ts", "./std/build/types/react-dom.d.ts"]
  }
);

And builds the following example to JS:

class Square extends React.Component {
  render() {
    return <button className="square">{/* TODO */}</button>;
  }
}

class Board extends React.Component {
  /**
   * @param {number} i
   */
  renderSquare(i) {
    return <Square />;
  }

  render() {
    const status = "Next player: X";

    return (
      <div>
        <div className="status">{status}</div>
        <div className="board-row">
          {this.renderSquare(0)}
          {this.renderSquare(1)}
          {this.renderSquare(2)}
        </div>
        <div className="board-row">
          {this.renderSquare(3)}
          {this.renderSquare(4)}
          {this.renderSquare(5)}
        </div>
        <div className="board-row">
          {this.renderSquare(6)}
          {this.renderSquare(7)}
          {this.renderSquare(8)}
        </div>
      </div>
    );
  }
}

class Game extends React.Component {
  render() {
    return (
      <div className="game">
        <div className="game-board">
          <Board />
        </div>
        <div className="game-info">
          <div>{/* status */}</div>
          <ol>{/* TODO */}</ol>
        </div>
      </div>
    );
  }
}

// ========================================

ReactDOM.render(<Game />, document.getElementById("root"));

@sholladay
Copy link

sholladay commented Mar 12, 2020

Regarding use cases, I use JSX for server-side rendering of HTML, not for bundling or use in a browser.

It's awesome that Deno supports it out of the box with zero configuration. Unfortunately, even a Hello World with JSX does not work anymore when used in a TypeScript file.

@kitsonk
Copy link
Contributor

kitsonk commented Mar 13, 2020

Ok, I am pretty sure that I understand how it broke. With .tsx and the move to strict meant that no any is allowed. Effectively to support .tsx we need to include some JSX intrinsic types as part of the runtime environment. I think I know what we need to do to fix it, and it feels like a legitimate use case, as we should support JSX out of the box (even though I personally hate JSX).

@sholladay do you use the Deno default of "jsx": "react" for your TSX?

@sholladay
Copy link

do you use the Deno default of "jsx": "react" for your TSX?

Yes, I'm not using any config. But also worth mentioning that I'm planning to use JSX in some reusable modules. Those projects obviously won't have control over the config used by the end-user.

@kitsonk
Copy link
Contributor

kitsonk commented Mar 13, 2020

@sholladay how are you making the React namespace available at runtime?

@sholladay
Copy link

sholladay commented Mar 13, 2020

Similar to the OP, I import React from the jspm CDN.

import React from 'https://dev.jspm.io/react@16.12.0';

The JSX, such as <h1>Hello, World!</h1>, gets rendered server-side (within Deno) to an HTML string and sent in a response using Deno's std/http server.

In the future, I will likely also try to use JSX for command line apps. Just need to make something like Ink for Deno.

@kitsonk
Copy link
Contributor

kitsonk commented Mar 13, 2020

The "problem" here is that we turned on strict mode, which is a good thing, but it means that TypeScript isn't allowing un-typesafe operations anymore. In typical workflows, people would install @types/react as one of their dependencies, and when they import React locally, the appropriate types load. TypeScript has two different things it looks for when dealing with JSX. The first is an interface called JSX.IntrinsicElements which provides the "standard" HTML elements. The second is React.createElement. Both of these are provided by @types/react (plus a lot of other types). (The types are "hacked" in the tests, which I didn't realise until now)

Given Deno, there are three possible solutions here I can see:

  1. We generate a Deno compatible set of React types that we host in std and people would use @deno-types to inform the compiler where the types are:
// @deno-types=https://deno.land/std/react/react.d.ts
import React from "https://dev.jspm.io/react@16.12.0";
  1. We get a CDN to provide it via X-TypeScript-Types (like https://pika.dev/cdn), then users would simply import from the CDN and it all would just work.
  2. We provide a set of JSX.IntrinsicElements types as part of the global scope as well as potentially React.createElement, which at least for the latter would be a "lie".

The first two options are likely the best options. Baking more stuff into Deno, especially stuff that might conflict with other use cases, doesn't make sense to me. I've got a set of types based of @types/react that work well under Deno that I can add to std.

@sholladay
Copy link

Those thoughts all sound reasonable to me. That said, I guess I don't really see the point of supporting JSX out of the box if it doesn't actually work. (That is, work under TypeScript, which is the norm for Deno.)

Maybe 2 is a good place to start, though.

@kitsonk
Copy link
Contributor

kitsonk commented Mar 14, 2020

The problem is JSX doesn't work out of the box anywhere. You always needs something to interpret it. Going with JSX preserve is totally useless in Deno, because it would emit code that just doesn't run. Therefore we default to React.createElement but it wouldn't make sense to include React.createElement in Deno. We could use some other type of factory, but that would still require us to include some sort of JSX code.

So the fact that JSX is useless in Deno without some sort of external library, means that we should get the needed types from that library. That means doing a import React from "..." in all the modules where you want to use JSX, which you were doing anyways for SSR.

So other we get more stronger opinionated ways of using JSX under Deno, or we just try to make it easier. The worry about that though is whatever opinions we have would mean it would be more difficult for someone to do something else if they didn't like those opinions.

@sholladay
Copy link

Could deno.land/x have an entry for React and provide the HTTP header for the types?

If not, I think my vote would be to include the JSX types globally. While it's opinionated, it's better than being broken.

@kitsonk
Copy link
Contributor

kitsonk commented Mar 14, 2020

Could deno.land/x have an entry for React and provide the HTTP header for the types?

Oooh, that is a good idea. In theory, it should be possible.

@sholladay
Copy link

sholladay commented Mar 17, 2020

Do we have an issue yet for providing the URL of the types via an HTTP header, so the compiler hint isn't necessary?

@kitsonk
Copy link
Contributor

kitsonk commented Mar 17, 2020

Now we do.

@EdAyers
Copy link

EdAyers commented May 16, 2020

I found it difficult to find the right imports and types to remove this error so
fwiw here is a working example as of 2020-05-16.

// main.tsx

// @deno-types="https://deno.land/x/types/react/v16.13.1/react.d.ts"
import React from "https://cdn.pika.dev/@pika/react@v16.13.1";

// @deno-types="https://deno.land/x/types/react-dom/v16.13.1/react-dom.d.ts"
import ReactDOM from "https://cdn.pika.dev/@pika/react-dom@v16.13.1";

// @deno-types="https://deno.land/x/types/react-dom/v16.13.1/server.d.ts"
import ReactDOMServer from "https://dev.jspm.io/react-dom@16.13.1/server.js";

const str: string = ReactDOMServer.renderToString(<div className="deno">land</div>);
console.log(str);
deno run main.tsx

@kolombet
Copy link

Those imports giving me a bunch of errors. Error messages not clear for me. Does it have problems with the downloading of modules or executing it, or it's a problem with the typings? I have absolutely no idea what is wrong with it. Deno 1.0.1
Screen Shot 2020-05-21 at 5 22 00 PM
Screen Shot 2020-05-21 at 5 22 12 PM

@bartlomieju
Copy link
Member

@kolombet this is probably bug introduced in #5029, see #5696 for details

@xcatliu
Copy link
Contributor

xcatliu commented May 25, 2020

@bardiarastin #5726 is published on deno v1.0.2
However, these code still report errors (but works fine on deno v1.0.0):

// main.tsx

// @deno-types="https://deno.land/x/types/react/v16.13.1/react.d.ts"
import React from "https://cdn.pika.dev/@pika/react@v16.13.1";

// @deno-types="https://deno.land/x/types/react-dom/v16.13.1/react-dom.d.ts"
import ReactDOM from "https://cdn.pika.dev/@pika/react-dom@v16.13.1";

// @deno-types="https://deno.land/x/types/react-dom/v16.13.1/server.d.ts"
import ReactDOMServer from "https://dev.jspm.io/react-dom@16.13.1/server.js";

const str: string = ReactDOMServer.renderToString(<div className="deno">land</div>);
console.log(str);
$ deno --version
deno 1.0.2
v8 8.4.300
typescript 3.9.2
$ deno run main.tsx                                        

Compile file:///Users/xcatliu/work/test/main.tsx
error: TS2307 [ERROR]: Cannot find module 'https://cdn.pika.dev/@pika/react@v16.13.1' or its corresponding type declarations.
import React from 'https://cdn.pika.dev/@pika/react@v16.13.1';
                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    at file:///Users/xcatliu/work/test/main.tsx:4:19

TS2307 [ERROR]: Cannot find module 'https://cdn.pika.dev/@pika/react-dom@v16.13.1' or its corresponding type declarations.
import ReactDOM from 'https://cdn.pika.dev/@pika/react-dom@v16.13.1';
                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    at file:///Users/xcatliu/work/test/main.tsx:7:22

TS2307 [ERROR]: Cannot find module 'https://dev.jspm.io/react-dom@16.13.1/server.js' or its corresponding type declarations.
import ReactDOMServer from 'https://dev.jspm.io/react-dom@16.13.1/server.js';
                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    at file:///Users/xcatliu/work/test/main.tsx:10:28

TS7026 [ERROR]: JSX element implicitly has type 'any' because no interface 'JSX.IntrinsicElements' exists.
const str: string = ReactDOMServer.renderToString(<div className="deno">land</div>);
                                                  ~~~~~~~~~~~~~~~~~~~~~~
    at file:///Users/xcatliu/work/test/main.tsx:12:51

TS7026 [ERROR]: JSX element implicitly has type 'any' because no interface 'JSX.IntrinsicElements' exists.
const str: string = ReactDOMServer.renderToString(<div className="deno">land</div>);
                                                                            ~~~~~~
    at file:///Users/xcatliu/work/test/main.tsx:12:77

Found 5 errors.

@xcatliu
Copy link
Contributor

xcatliu commented Jun 1, 2020

Fixed in Deno v1.0.3

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

Successfully merging a pull request may close this issue.

8 participants