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

Add option to disable jsx -> htm conversion #93

Open
marvinhagemeister opened this issue Sep 21, 2020 · 6 comments
Open

Add option to disable jsx -> htm conversion #93

marvinhagemeister opened this issue Sep 21, 2020 · 6 comments
Labels
enhancement New feature or request
Milestone

Comments

@marvinhagemeister
Copy link
Member

marvinhagemeister commented Sep 21, 2020

I'm using wmr in the test suite for our Preact Devtools extension and I need to verify that it works with a range of Preact versions.

To do so I've set up multiple aliases:

{
  "alias": {
    "preact@10.0.0-rc.0": "/vendor/preact-10.0.0-rc.0/preact.js",
    "preact@10.3.4": "/vendor/preact.10.3.4/preact.js"
}

And than we can use that with standard imports:

import { h, render } from "preact@10.3.4";

render(
  <p>Rendered with 10.3.4</p>,
  document.getElementById("app")
);

This works really well except for when I use JSX. JSX is transpiled to htm template strings, but by adding a import htm from "htm/preact" import, an additional version of Preact is pulled in.

import { html } from "/@npm/htm/preact"; // Pulls in wrong version
import { h, render } from "preact@10.3.4";

render(
  html`<p>Rendered with 10.3.4</p>`,
  document.getElementById("app")
);

To solve this I can come up with two options:

  1. Transpile JSX to createElement calls

This is arguable the more verbose version, but it works. The parsing performance benefit of tagged template strings doesn't really come to play in my set up, so I'm good with createElement calls.

import { h, render } from "preact@10.3.4";

render(
  h("p", null, "Rendered with 10.3.4"),
  document.getElementById("app")
);
  1. Inject htm.bind(h) calls into every file

Another option could be to drop the htm/preact import and initialize the htm constructor anew in each file:

import htm from "/@npm/htm";
import { h, render } from "preact@10.3.4";

const html = htm.bind(h);

render(
  html`<p>Rendered with 10.3.4</p>`,
  document.getElementById("app")
);
  1. Transpile to html calls, but don't inject htm.bind(h) automatically

This closely ties to 2) and is probably a lot more flexible for the end user. Essentially he'd need to make sure that an html function is present in the module scope themselves.

@developit
Copy link
Member

Ooooh so this is 100% a bug in the npm resolution. Does it happen on master?

@marvinhagemeister
Copy link
Member Author

Yeah happens on master too. Not sure if it's a resolution issue too. htm can't know which preact version is used on the page. I'm passing the version via an url parameter so that I can run a test against multiple preact versions. So wmr is correct in resolving the preact import inside htm/preact via node resolution to node_modules.

@developit
Copy link
Member

Ah interesting, yeah that's kinda a funky case. A thought: what if we had the HTM plugin detect /** @jsx h */ at the top of files and insert htm.bind(h)? The default would be to pull in htm/preact`, but any file could opt out of it.

@marvinhagemeister
Copy link
Member Author

@developit ohh I like that very much! That's much better than my proposed solution!! 👍

@developit
Copy link
Member

I just thought of another idea: we could use the new jsxRuntime comment option from Babel to generate a module that pulls in a different h()/jsx().

@developit developit added this to To do in 1.0 via automation Sep 30, 2020
@marvinhagemeister
Copy link
Member Author

@developit That sounds even better and more future proof. Love that 👍

@developit developit added the enhancement New feature or request label Oct 1, 2020
@developit developit removed this from To do in 1.0 Nov 29, 2020
@developit developit modified the milestones: 1.1, 1.2 Nov 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
2 participants