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

Proposal: [breaking-change] Default to h and Fragment for JSX #11186

Closed
kitsonk opened this issue Jun 30, 2021 · 9 comments
Closed

Proposal: [breaking-change] Default to h and Fragment for JSX #11186

kitsonk opened this issue Jun 30, 2021 · 9 comments
Labels
breaking change a change or feature that breaks existing semantics cli related to cli/ dir suggestion suggestions for new features (yet to be agreed)
Milestone

Comments

@kitsonk
Copy link
Contributor

kitsonk commented Jun 30, 2021

Currently, Deno CLI defaults to effectively the following compiler options for TypeScript:

{
  "compilerOptions": {
    "jsx": "react",
    "jsxFactory": "React.createElement",
    "jsxFragmentFactory": "React.Fragment",
  }
}

I believe it is far more common though in Deno to not use React specifically for server-side rendering, the most common use case for JSX/TSX in Deno. Instead I think modular libraries where you import h and Fragment are far more common (Like Preact or Nano-JSX).

I propose we default to the following settings:

{
  "compilerOptions": {
    "jsx": "react",
    "jsxFactory": "h",
    "jsxFragmentFactory": "Fragment",
  }
}

Currently, anyone using one of these "importable" libraries has to either use the --config option on the command line, or use the TypeScript JSX pragma in each file:

/** @jsx h */
/** @jsxFragment Fragment */

Considerations

  • Deno Deploy uses h and Fragment and is only overridable via the pragmas.
  • There are the "new" React transforms which we need to support (see: Support new JSX transforms #8440)
  • Considering it is a breaking change, should we introduce a flag to restore the old behaviour, like --jsx-react or just people to use --config or the pragma? I am for documenting it and encouraging people to use --config or the pragma if they want the old behaviour.
@kitsonk kitsonk added cli related to cli/ dir breaking change a change or feature that breaks existing semantics proposal labels Jun 30, 2021
@ry
Copy link
Member

ry commented Jul 1, 2021

Ideally we could have a deno_lint rule that would appropriate warn people of this upcoming change.

Unfortunately it seems like deno_lint does not take into account tsconfig yet.

@bartlomieju
Copy link
Member

Ideally we could have a deno_lint rule that would appropriate warn people of this upcoming change.

Unfortunately it seems like deno_lint does not take into account tsconfig yet.

It's a low hanging fruit to pass tsconfig options to deno_lint

@bartlomieju
Copy link
Member

Opened an issue in deno_lint repo. To make it work, deno lint subcommand should accept --config flag as well.

@kitsonk
Copy link
Contributor Author

kitsonk commented Jul 8, 2021

One suggestion on this has been to "turn off" JSX by default and to encourage people to use the pragmas of @jsx, @jsxFrag and @jsxImportSource on a per file basis (or maybe even just @jsxImportSource). Some considerations on this would be:

  • Does turning it off and "locking" people out increase or decrease portability of code?
  • Does encouraging people to use the pragmas on a per file basis increase or decrease portability?
  • The new transforms (Support new JSX transforms #8440) import an extension-less import, and everyone just seems fine with that. We can resolve that magic.
  • Nano-JSX (a lightweight JSX library that targets Deno and NodeJS) doesn't currently support it (see: Consider supporting the React JSX transform API nanojsx/nano#20).

@stale
Copy link

stale bot commented Sep 6, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Sep 6, 2021
@kitsonk kitsonk added the suggestion suggestions for new features (yet to be agreed) label Sep 6, 2021
@stale stale bot removed the stale label Sep 6, 2021
@kitsonk kitsonk mentioned this issue Sep 17, 2021
17 tasks
@kitsonk kitsonk self-assigned this Sep 17, 2021
@kitsonk kitsonk added this to the 2.0.0 milestone Sep 17, 2021
@marvinhagemeister
Copy link
Contributor

@bartlomieju I think this can be closed. Pretty much everyone switched to the newer automatic runtime transform where you specify a single jsxImportSource value. One could argue that changing this default from react to something else could be done, but given that every other JSX transpiler defaults to react we shouldn't deviate from the ecosystem.

@bartlomieju
Copy link
Member

@marvinhagemeister we discussed this yesterday and David wants to change it.

@marvinhagemeister
Copy link
Contributor

Oh in that case I don't mind. Happy it assist in any way possible 👍

@bartlomieju
Copy link
Member

Discussed with Luca and David and we feel it's not worth the effort to switch these as the most used way to configure JSX is jsxImportSource these days as pointed out by Marvin in (#11186 (comment)).

Closing without further action.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change a change or feature that breaks existing semantics cli related to cli/ dir suggestion suggestions for new features (yet to be agreed)
Projects
None yet
Development

No branches or pull requests

5 participants