feat(typescript): add support for "jsx: preserve" compiler option #2574
Conversation
duarten
requested review from
alexeagle,
gregmagolan,
mattem and
soldair
as code owners
| load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_test") | ||
| load("//packages/typescript:index.bzl", "ts_project") | ||
|
|
||
| # Ensure that a.js produces outDir/a.js, outDir/a.d.ts, and outDir/a.d.ts.map |
There was a problem hiding this comment.
comment looks out-of-date
| # Ensure that a.js produces outDir/a.js, outDir/a.d.ts, and outDir/a.d.ts.map | ||
| SRCS = [ | ||
| "a.tsx", | ||
| "b.jsx", |
There was a problem hiding this comment.
I think the second file is just redundant? reduce the test case to one file if that gives the same assertions/coverage
There was a problem hiding this comment.
The idea of the second file is to assert that jsx files are not transformed to js files (which happens if jsx is not set to preserve).
| @@ -0,0 +1,10 @@ | |||
| const assert = require('assert'); | |||
|
|
|||
| console.log(process.argv) | |||
| const files = process.argv.slice(2); | ||
| assert.ok(files.some(f => f.endsWith('out/a.d.ts')), 'Missing a.d.ts'); | ||
| assert.ok(files.some(f => f.endsWith('out/a.d.ts.map')), 'Missing a.d.ts.map'); | ||
| assert.ok(files.some(f => f.endsWith('out/a.jsx'), 'Missing a.jsx.map')); |
There was a problem hiding this comment.
missed some copy-paste here, the endsWith doesn't match the message
| if (option === 'jsx') { | ||
| return jsxEmit[options[option] as ts.JsxEmit] | ||
| } | ||
| if (typeof (options[option]) === 'string' && option !== 'jsx') { |
There was a problem hiding this comment.
else if rather than repeat the inverse of the jsx condition
duarten
force-pushed
the
feature/jsx-preserve
branch
2 times, most recently
from
8f321bf
to
f6e5fbe
Compare
|
@alexeagle Trying to get it to green :) I actually just refactored the approach a bit, there's now a "preserve_jsx" attribute instead of "jsx", since we just care about the "preserve" option. |
duarten
force-pushed
the
feature/jsx-preserve
branch
from
f6e5fbe
to
317e697
Compare
When tsc is instructed to preserve jsx, the emitted files have a .jsx extension for .tsx and .jsx inputs, which means ts_project needs to be aware of the option to properly define outputs.
duarten
force-pushed
the
feature/jsx-preserve
branch
from
317e697
to
c0300cb
Compare
There was a problem hiding this comment.
at first my reaction was "it's semantically similar to just map jsx=foo from tsconfig into BUILD" but then thinking more, the common case of jsx=react would force people to add a line to their BUILD file. So I like your boolean attribute more.
Thanks for the fix!!
When tsc is instructed to preserve jsx, the emitted files have a .jsx
extension for .tsx and .jsx inputs, which means ts_project needs to be
aware of the option to properly define outputs.
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
The current behavior causes the
ts_projectrule to fail as it is expecting different outputs.Issue Number: N/A
What is the new behavior?
In the new behavior,
ts_projectis aware of thejsxcompiler option and sets output files accordingly.Does this PR introduce a breaking change?
Other information