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

jsx set to "preserve" still applies transforms to JSX #3605

Closed
martypdx opened this issue Jan 21, 2024 · 5 comments
Closed

jsx set to "preserve" still applies transforms to JSX #3605

martypdx opened this issue Jan 21, 2024 · 5 comments

Comments

@martypdx
Copy link

martypdx commented Jan 21, 2024

Working reproduction

Esbuild with these inputs:

  • option: jsx: 'preserve, loader: 'tsx'
  • transform jsx that contains JsxText AST Nodes:
    <li className={"item"}>Hello {world}??</li>

Expected output is that jsx is "preserved" as-is:

    <li className={"item"}>Hello {'world'}??</li>

Actual output transforms JSXText to JSXExpressionContainer (and removes JSXExpressionContainer from literal attirbute. Also introducing additional formatring which has downstream effect of introducing additional text nodes:

<li className="item">
  {"Hello "}
  {world}
  {"??"}
</li>;

(LMK if I am just missing another setting!)

AFAICT, here is where the transform is applied.

IMO jsx: 'preserve' should not make any JSX transforms that would result in a different JSX AST being produced from the output what was produced for the jsx from the source.

Happy to help with test/PR if needed.

@martypdx martypdx changed the title Jsx "preserve" applies transforms to JSXText jsx set to "preserve" still applies transforms to JSX Jan 21, 2024
@martypdx
Copy link
Author

martypdx commented Jan 21, 2024

I created the same jsx transform in tsc compiler with jsx preserve.

tsc does seem to be significant whitespace aware. They probably take the approach of outputting the raw value from the AST for JSXText nodes.

Is there an opportunity to skip iterative JSX generation entirely for preserve mode and somehow splice in the jsx segments into the output from the AST locations?

@evanw
Copy link
Owner

evanw commented Jan 21, 2024

IMO jsx: 'preserve' should not make any JSX transforms that would result in a different JSX AST being produced from the output what was produced for the jsx from the source.

The reason this is happening is because esbuild is parsing JSX into an AST and then printing it back out. In other words, the ASTs that esbuild makes for both the input and the output are exactly the same, just printed differently. You can see here that both the input and the output are equivalent when transformed into JS: (link)

You haven't described an actual problem you are having. Why is this a problem for you?

@martypdx
Copy link
Author

martypdx commented Jan 22, 2024

Sure thing! I am applying an ahead-of-time compile transformation to JSX directly after the esbuild step in vite.It works great to have esbuild handle the typescript to jsx conversion, then I use acorn+jsx parser to get the ast and handle the custom generation, hmr, etc. Its a fast compilation pipeline.

The difference in jsx output messes with the subsequent html generation and binding targets. It ends up reversing the optimizations of moving html content out of js files and multiplies the number of binding targets generated.

This jsx:

const render = () => <li className={category}>Hello {place}</li>;

Generates this html/js:

<li data-bind>Hello <!--0--></li>
const render = () => {
    const { node: te19fd83eae, targets: __targets } = __rendererById('e19fd83eae');
    const __target0 = __targets[0];
    const __child1 = __target0.childNodes[1];
    __target0.className = (category);
    __compose(place, __child1);
    return te19fd83eae;
};

While this jsx:

const render = () => <li className={category}>
  {"Hello "}
  {place}
</li>;

generates this html/js:

<li data-bind="">
  <!--0-->
  <!--0-->
</li>
const render = () => {
  const { node: t43944ca1c9, targets: __targets } = __rendererById('43944ca1c9');
  const __target0 = __targets[0];
  const __child1 = __target0.childNodes[1];
  const __child2 = __target0.childNodes[3];
  __target0.className = (category);
  __compose("Hello ", __child1);
  __compose(place, __child2);
  return t43944ca1c9;
};

@evanw
Copy link
Owner

evanw commented Jan 23, 2024

I see. My intuition as a developer is that those two cases would be equivalent since I'd expect any relevant optimizations to be applied regardless of how the source code was written (e.g. <a href={foo}/> may become <a href={"inline string"}/> after an optimizer inlines it which I'd expect to be just as optimized as <a href="inline string"/>).

But I can still see an argument for changing esbuild's behavior here. The intent behind the word "preserve" is "doesn't change" and if someone wants to make <a href="inline string"/> mean something different than <a href={"inline string"}/>, esbuild shouldn't get in your way. The JSX specification deliberately doesn't specify how JSX is to be interpreted so pretty much anything goes. I'll make this change to esbuild.

One random caveat with this is that the charset setting will no longer affect the content of JSX text if JSX text is now reproduced verbatim. This seems fine though since the purpose of the charset setting is to work around with servers not setting the encoding correctly and JSX needs another transform before being served anyway, so the responsibility of the encoding workaround lies with that additional tool instead of with esbuild.

@evanw evanw closed this as completed in e04a690 Jan 23, 2024
@martypdx
Copy link
Author

martypdx commented Jan 24, 2024

My intuition as a developer is that those two cases would be equivalent since I'd expect any relevant optimizations to be applied regardless of how the source code was written (e.g. may become <a href={"inline string"}/> after an optimizer inlines it which I'd expect to be just as optimized as ).

Intuition shaped by vdom architecture. 😁 For a render-once architecture you want to offload as much into the static initial html because then it is processed by the browser and not the library/fw. <div>some text here</div> to <div>{'some text here'}</div> is vastly different because the former is inlined into html that the browser handles whereas the later has to be managed via javascript by the library/framework. The attribute example you gave would be a good optimization because it would make the attribute part of the static clonable structure of the html template versus a binding that needs to be managed in javascript.

I was really impressed with how well esbuild does static optimization and inlining. It was hard to make a test case to "fool" it into outputting variables instead of static text. Even module boundaries are no obstacle! I am going to look at using that capability to try and "bundle" trees of components into equivalent single components.

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

No branches or pull requests

2 participants