Add JSX support built on the JS backend #21206

tobni · 2024-07-25T11:59:18Z

This is a small change for the JS backend, as the tree-sitter parser already supports JSX.

This is a fairly big decision w.r.t how the js backend will evolve with typescript and other javascript transpile formats. Since many tools that support js also supports other kinds of files, the JSRuntime... will be used to refer to the files that can be treated as a source code file for some node-ish runtime who either processes the AST or executes actual programmes.

Clarifiying examples:

A linter that can process any js-esque file should accept JSRuntimeSourceField
All test sources should use JSRuntimeDependencies
Files that can cross import (post bundling) different transpile formats should use JSRuntimeDependencies and JSRuntimeSourceField. Think typescript, JSX.
A checker that can only process typed vue-templates should not accept JSRuntimeSourceField.

This PR together with #21176 essentially gives pants "react" support.

tobni · 2024-07-25T12:02:45Z

The motivation for providing a separate target is that plugin authors can distinguish between the sources.

huonw

Thanks so much for continuing to plug away at this work! This is awesome

This PR together with #21176 essentially gives pants "react" support.

🎉

Files that can cross import (post bundling) different transpile formats should use JSRuntimeDependencies and JSRuntimeSourceField. Think typescript, JSX.

More questions to exploring the problem space:

AIUI, bundlers allow importing other resources too, e.g. import './example.css' or even images (https://vitejs.dev/guide/assets), do we have thoughts on how those might be modelled? Maybe as an inferred JSRuntimeDependencies on a file or resource target? Or maybe something else?
How does pants test path/to/some.test.jsx work? I see the test project has a test JSX-using test file in it, but it's not obvious to me how this actually executes? I'd be expecting more plumbing to make that work, so I'm surprised it's not here!

docs/notes/2.23.x.md

src/python/pants/backend/jsx/target_types.py

huonw · 2024-07-26T05:25:33Z

src/python/pants/backend/javascript/lint/prettier/rules.py

-    required_fields = (JSSourceField,)
+    required_fields = (JSRuntimeSourceField,)


On the target discussion, for prettier specifically, it supports things that are very non-JS-shaped, e.g. HTML, GraphQL, Markdown, JSON, Yaml.

Two questions:

Specifically for prettier: do we currently have any thoughts on how to make Pants able to use Prettier to (optionally!) format non-JS files too?

Generalising/another clarifying example: are there other tools in the JS ecosystem that might behave similarly and work with more file types? Do we need to think about how to handle them?

No. I have no use case myself but I suppose a contributor could open up the prettier plugin to more languages later by adding a union-layer? Alternatively prettier seems like the kind of tool one might want to run "target-less" and just glob?

The typescript compiler can check other files than .ts. It can be run on .js, .jsx and .tsx as well Test runners have the same capability, so the test goal also applies. Those are the usecases I'm primarily intrested in, and I think the "JS-ish source" model works well there.

I also think it is fun that you implied json is non-js shaped 😏 I get what you're saying though, you do not run a json file.

Alternatively prettier seems like the kind of tool one might want to run "target-less" and just glob?

Hm, maybe, yeah.

The typescript compiler can check other files than .ts. It can be run on .js, .jsx and .tsx as well Test runners have the same capability, so the test goal also applies. Those are the usecases I'm primarily intrested in, and I think the "JS-ish source" model works well there.

👍

I think I wrote the Prettier plugin a while back, and we did have the discussion about how it could be used target-less, but that (d)evolved into a much larger discussion about the needs for targets and target-less and so on and so forth.

There are a few tools where I think we just want to defer to whatever the tool can handle - and I don't know how we do that in the most pants-esque way

huonw · 2024-07-26T05:30:34Z

src/python/pants/backend/javascript/target_types.py

    pass


-class JSSourceField(SingleSourceField):
+class JSRuntimeSourceField(SingleSourceField):
+    """A source that is javascript at runtime."""


Feeding into the PR description. Thinking about this framing: AIUI, there's some tools that can run TypeScript natively (e.g. Bun; Node (experimentally) as of nodejs/node#53725), so a .ts file can be run without being/becoming JS.

Does that play into the broader consideration, or maybe it's not a big-picture issue (e.g. maybe just this doc string is (very slightly) inaccurate)?

I suppose it is a point of semantics, but bun and node do not run typescript. They strip (node) or compile-down the (bun) types and run the resulting javascript.

Yeah, I think my key point is that there's tools for which the file run is the .ts one, without a bundling/packaging/compilation step to turn it into real JS first, so it's becomes harder to say that it "is Javascript" at runtime.

Brainstorming: I wonder if we could instead flip this around to be tool-focused using a union somehow (I'm not fully familiar with whether that's possible, though). E.g. register various source types as members of "can be passed to tsc ...", "can be passed to node ...", "prettier ...", "biome ...", etc.

For specific tools were we can have a field set and union to target the tool, yes.

Not sure how that works out for node_test_script. The TestFieldSet is already a union that requires a SourcesField unique to each union member.

In other words, I think at some point we are required to "boil down" to a unique source field type to "end up" with a JSTestRequest. If it was generic on e.g SourceField I think the union is not specialized enough? But idk 🤷‍♂️

In other words, I think at some point we are required to "boil down" to a unique source field type to "end up" with a JSTestRequest. If it was generic on e.g SourceField I think the union is not specialized enough? But idk 🤷‍♂️

IDK either 🤷‍♂️

.gitignore

testprojects/src/js/hello_react/src/test/index.test.js

tobni · 2024-07-27T20:59:31Z

Thanks so much for continuing to plug away at this work! This is awesome

This PR together with #21176 essentially gives pants "react" support.

🎉

Files that can cross import (post bundling) different transpile formats should use JSRuntimeDependencies and JSRuntimeSourceField. Think typescript, JSX.

More questions to exploring the problem space:

AIUI, bundlers allow importing other resources too, e.g. import './example.css' or even images (https://vitejs.dev/guide/assets), do we have thoughts on how those might be modelled? Maybe as an inferred JSRuntimeDependencies on a file or resource target? Or maybe something else?

How does pants test path/to/some.test.jsx work? I see the test project has a test JSX-using test file in it, but it's not obvious to me how this actually executes? I'd be expecting more plumbing to make that work, so I'm surprised it's not here!

Yes, that is true. For now I believe pants will pull in resources and files into js-backend-sandboxes as long as there is a dependency to them. That dependency is inferred OOTB with javascript: jsconfig (tsconfig) parsing for dependency inference #21176, because I didn't filter the target sources on any specific field (but you have to include extensions e.g .css as the extension substitution is only for js).
I missed this because I've never seen jsx tests before (even though I just wrote one in this pr 🤪 ). The common case is to probe your transpiled JSX using some JS-framework, you rarely write markup in tests... But it is for sure a thing so test-targets should be included in this PR. The "plumbing" required should only be that the target in question inherits the yet to exist JSRuntimeTestSourceField analogous with the JSRuntimeSourceField, I guess.

riisi · 2024-07-30T23:49:50Z

Excited for this 🎉

suppose it is a point of semantics, but bun and node do not run typescript.

Just a heads up that it looks like Node will soon run typescript - nodejs/node#53725

tobni · 2024-07-31T20:23:31Z

src/python/pants/backend/jsx/target_types.py

+class JSXTestDependenciesField(JSXDependenciesField):
+    pass
+
+
+class JSXTestSourceField(JSXSourceField, JSTestRuntimeSourceField):
+    expected_file_extensions = JSX_FILE_EXTENSIONS
+
+
+class JSXTestTimeoutField(TestTimeoutField):
+    pass
+
+
+class JSXTestExtraEnvVarsField(TestExtraEnvVarsField):
+    pass


I'm honestly not sure why I declare subclasses for each field, but it seems prevalent in all backends. A lot of boilerplate, though.

docs/notes/2.23.x.md

Co-authored-by: riisi <rhysmadigan@gmail.com>

huonw

Looks good.

Approving to keep us moving forward, because it seems like the best way to explore the best option for how to model these targets is to starting doing something and work out if we like it or not!

huonw · 2024-08-04T22:22:29Z

src/python/pants/backend/javascript/target_types.py

    pass


-class JSSourceField(SingleSourceField):
+class JSRuntimeSourceField(SingleSourceField):
+    """A source that is javascript at runtime."""


In other words, I think at some point we are required to "boil down" to a unique source field type to "end up" with a JSTestRequest. If it was generic on e.g SourceField I think the union is not specialized enough? But idk 🤷‍♂️

IDK either 🤷‍♂️

tobni added backend: JavaScript JavaScript backend-related issues category:new feature labels Jul 25, 2024

tobni self-assigned this Jul 25, 2024

tobni requested a review from sureshjoshi July 25, 2024 12:05

tobni added 2 commits July 25, 2024 15:02

feat: Add JSX support

3330c4a

doc: Add release note

3cb9365

tobni force-pushed the add/jsx branch from 4ed128b to 3cb9365 Compare July 25, 2024 13:02

huonw reviewed Jul 26, 2024

View reviewed changes

tobni added 4 commits July 31, 2024 21:15

chore: add newline

de30fd3

feat: Enable JSX with JS backend

6676f7c

feat: Add JSX test targets and make them targetable

4efaacb

docs: Correct docstring

9bab4e1

tobni force-pushed the add/jsx branch from 9502b7c to 9bab4e1 Compare July 31, 2024 20:11

tobni commented Jul 31, 2024

View reviewed changes

tobni requested a review from huonw August 1, 2024 07:48

tobni added 5 commits August 3, 2024 12:53

Merge remote-tracking branch 'upstream/main' into add/jsx

56caa50

test: Apply common bundler settings to test project

6735f2d

chore: Run fmt

d1db61c

docs: Correct note now that JSX is included in JS

55be737

chore: Add newline

74c8290

riisi reviewed Aug 3, 2024

View reviewed changes

docs/notes/2.23.x.md Outdated Show resolved Hide resolved

docs: capitalization

3b882c7

Co-authored-by: riisi <rhysmadigan@gmail.com>

huonw approved these changes Aug 4, 2024

View reviewed changes

tobni merged commit 6c41074 into pantsbuild:main Aug 7, 2024
25 checks passed

huonw mentioned this pull request Sep 2, 2024

Implement complementary TSX backend #21327

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add JSX support built on the JS backend #21206

Add JSX support built on the JS backend #21206

tobni commented Jul 25, 2024

tobni commented Jul 25, 2024

huonw left a comment

huonw Jul 26, 2024

tobni Jul 27, 2024 •

edited

Loading

tobni Jul 27, 2024 •

edited

Loading

huonw Jul 28, 2024

sureshjoshi Jul 28, 2024

huonw Jul 26, 2024

tobni Jul 27, 2024 •

edited

Loading

huonw Jul 28, 2024

tobni Jul 28, 2024 •

edited

Loading

huonw Aug 4, 2024

tobni commented Jul 27, 2024 •

edited

Loading

riisi commented Jul 30, 2024

tobni Jul 31, 2024 •

edited

Loading

huonw left a comment

huonw Aug 4, 2024

		required_fields = (JSSourceField,)
		required_fields = (JSRuntimeSourceField,)

Add JSX support built on the JS backend #21206

Add JSX support built on the JS backend #21206

Conversation

tobni commented Jul 25, 2024

tobni commented Jul 25, 2024

huonw left a comment

Choose a reason for hiding this comment

huonw Jul 26, 2024

Choose a reason for hiding this comment

tobni Jul 27, 2024 • edited Loading

Choose a reason for hiding this comment

tobni Jul 27, 2024 • edited Loading

Choose a reason for hiding this comment

huonw Jul 28, 2024

Choose a reason for hiding this comment

sureshjoshi Jul 28, 2024

Choose a reason for hiding this comment

huonw Jul 26, 2024

Choose a reason for hiding this comment

tobni Jul 27, 2024 • edited Loading

Choose a reason for hiding this comment

huonw Jul 28, 2024

Choose a reason for hiding this comment

tobni Jul 28, 2024 • edited Loading

Choose a reason for hiding this comment

huonw Aug 4, 2024

Choose a reason for hiding this comment

tobni commented Jul 27, 2024 • edited Loading

riisi commented Jul 30, 2024

tobni Jul 31, 2024 • edited Loading

Choose a reason for hiding this comment

huonw left a comment

Choose a reason for hiding this comment

huonw Aug 4, 2024

Choose a reason for hiding this comment

tobni Jul 27, 2024 •

edited

Loading

tobni Jul 27, 2024 •

edited

Loading

tobni Jul 27, 2024 •

edited

Loading

tobni Jul 28, 2024 •

edited

Loading

tobni commented Jul 27, 2024 •

edited

Loading

tobni Jul 31, 2024 •

edited

Loading