V2: Typescript transform support through Babel (default) and tsc #3083

wbinnssmith · 2019-05-24T20:24:55Z

Using @DeMoorJasper's work in #2627 as a base, this implements the ability for Parcel 2 to transform Typescript. It implements it by default using Babel rather than using tsc, but also implements a TSC transformer which uses TSC instead.

For tsc, a config-tsc is supplied with Parcel which runs typescript code through tsc but not through babel.

Open questions:

Is it common to run code through both tsc and babel? ~~Currently this isn't possible.~~ Actually, it probably is! Though the babel transform would run with the flow-strip-types plugin enabled, but that shouldn't interfere once TS is compiled to JS.
Right now, mimicking what we do with Babel, we decide to use the user's typescript if we find a tsconfig. Since it's possible to use tsc without one, what should we do if the user has a local copy of typescript?
How should we best test the ability to use the user's version of typescript? From an integration standpoint, it seems very difficult to tell which version of typescript a module was transpired with, as tsc's transpileModule is very loose and does not throw when it encounters unknown syntax.

Typescript-powered resolving will follow in another PR.

Test Plan: The same integration tests test both Babel and tsc transformers. yarn test

wbinnssmith · 2019-05-24T20:43:26Z

packages/configs/tsc/index.json

+  "transforms": {
+    "*.{ts,tsx}": [
+      "@parcel/transformer-typescript-tsc",
+      "@parcel/transformer-js",


should we also run babel here? Might be surprising for TS users to always have their code run through babel, though it would be limited to preset-env, flow-strip-types (unnecessarily), and preset-react (unnecessarily)

Thought we only added flow-strip-types if we saw // @flow and something similar for React? babel-preset-env seems like it would still be valuable unless tsc is capable of something similar.

wbinnssmith · 2019-05-24T21:18:11Z

packages/transformers/typescript-tsc/src/TSCTransformer.js

+
+export default new Transformer({
+  async getConfig({asset}) {
+    return asset.getConfig(['tsconfig.json']);


This should use the compiler api rather than our own [0] like @fathyb's typescript plugin [1]
We should also make sure it resolves extends fields present in configs -- the version here certainly won't.

[0] https://github.com/Microsoft/TypeScript/wiki/Using-the-Compiler-API
[1] https://github.com/fathyb/parcel-plugin-typescript/blob/f3045b7823a01e69db9288676c7876e001182372/src/backend/config-loader.ts#L29

cc @padmaia

Looks like Fathy's plugin resolves the initial config and passes it to the compiler options. Is there something keeping you from doing something similar right now? Also I wonder if the compiler tells us which files it extended. Probably not as most tools don't, but I wonder if it would be difficult to get that feature in. Otherwise we wouldn't be able to rebuild in watch mode if one of the extended files changed and would have to rerun loading the tsconfig for every file whose tsconfig had an extends on start up 😕 (Same as babel, @wbinnssmith you already know most of this, just commenting for posterity)

devongovett · 2019-05-25T00:46:53Z

I wonder if we could avoid having a separate parcel config in order to use tsc. We could do this by separating the babel transformer for typescript into a separate parcel plugin from the babel transformer.

{
  "transformers": {
    "*.{ts,tsx}" : [
      "@parcel/transformer-babel-typescript",
      "@parcel/transformer-tsc"
    ],
    "*.{js,jsx}": [
      "@parcel/transformer-babel",
      "@parcel/transformer-js"
    ]
  }
}

With this config, @parcel/transformer-babel-typescript would run first. It could look for a tsconfig.json config, and if one is found, just do nothing. In that case, @parcel/transformer-tsc would run. Otherwise, if there was no tsconfig.json, @parcel/transformer-babel-typescript would run, and the asset would jump to the js pipeline without running @parcel/transformer-tsc (the asset type changed).

Because we have AST sharing, there shouldn't be a performance penalty for separating @parcel/transformer-babel-typescript from @parcel/transformer-babel (although maybe babel could combine other babel plugins into the same traversal?). There could be a small perf hit when using TSC since the tsconfig.json would still need to be resovled by @parcel/transformer-babel-typescript before TSC even runs, but perhaps config caching will solve that as well.

As an alternative to separating it out, perhaps @parcel/transformer-babel could replace @parcel/transformer-babel-typescript in the config above, and do the same thing. But then, the rest of the JS pipeline would need to be duplicated into the TS pipeline, in order to prevent babel from running twice.

devongovett · 2019-05-25T00:47:54Z

packages/resolvers/default/src/DefaultResolver.js

@@ -18,7 +18,7 @@ import builtins from './builtins';
 export default new Resolver({
  async resolve({dependency, options}) {
    const resolved = await new NodeResolver({
-      extensions: ['js', 'json', 'css'],
+      extensions: ['js', 'ts', 'tsx', 'json', 'css'],


oh yeah... we'll need to derive this list from the config somehow...

DeMoorJasper · 2019-05-28T20:10:48Z

I'm not sure if we should run tsc by detecting a tsconfig.json, it could be unwanted behaviour.
In most projects I have worked on the tsconfig.json file is used for type checking and IDE integration while compilation is still done using Babel.

wbinnssmith · 2019-06-01T22:17:28Z

In most projects I have work(ed) on the tsconfig.json file is used for type checking and IDE integration while compilation is still done using Babel.

This has been my experience too, and why I think unfortunately @devongovett's direction above might not work out.

mischnic · 2019-06-02T10:55:08Z

packages/transformers/typescript-tsc/src/TSCTransformer.js

+    if (config) {
+      transpilerOptions.compilerOptions = {
+        ...transpilerOptions.compilerOptions,
+        ...config.compilerOptions


compilerOptions.module should not be overwritable by the user config if scope hoisting is enabled (has caused issues in the past)

wbinnssmith · 2019-06-03T23:24:34Z

What's needed here that's preventing this from landing?

mischnic · 2019-06-04T07:27:23Z

How do you determine whether to use babel or tsc?

wbinnssmith · 2019-06-04T18:27:17Z

How do you determine whether to use babel or tsc?

My proposal is that we don't, because as @DeMoorJasper mentioned, we can't. We default to babel, and the user opts into tsc by using a different parcel configuration.

mischnic · 2019-06-04T18:30:36Z

We default to babel, and the user opts into tsc by using a different parcel configuration.

Ah, I missed that in the changes somehow 👍

dgreensp · 2019-06-07T19:25:52Z

I just want to lend my support for the direction here. I think the flexibility is fantastic. It is super useful to be able to choose to run your TypeScript through either:

just TSC;
TSC followed by Babel; or
just Babel

Keep it up.

The ability for Parcel to show TS type errors would be pretty useful, as well.

mischnic · 2019-06-08T20:39:23Z

I get:

🚨 Got unexpected undefined
    at nullthrows /parcel/node_modules/nullthrows/nullthrows.js:7:15)
    at CallExpression /parcel/packages/shared/scope-hoisting/src/link.js:166:21)
    at NodePath._call /parcel/node_modules/@babel/traverse/lib/path/context.js:53:20)
    at NodePath.call /parcel/node_modules/@babel/traverse/lib/path/context.js:40:17)
    at NodePath.visit /parcel/node_modules/@babel/traverse/lib/path/context.js:88:12)
    at TraversalContext.visitQueue /parcel/node_modules/@babel/traverse/lib/context.js:118:16)
    at TraversalContext.visitSingle /parcel/node_modules/@babel/traverse/lib/context.js:90:19)
    at TraversalContext.visit /parcel/node_modules/@babel/traverse/lib/context.js:146:19)
    at Function.traverse.node /parcel/node_modules/@babel/traverse/lib/index.js:94:17)
    at NodePath.visit /parcel/node_modules/@babel/traverse/lib/path/context.js:95:18)

with the simple example (renaming all js to ts).

wbinnssmith · 2019-06-10T20:28:56Z

I get:
with the simple example (renaming all js to ts).

Weird, I don't get that at all. Can you send by a tarball of the transformed example you used?

wbinnssmith · 2019-06-10T20:30:58Z

Ah, got it. We don't run through scopehoisting outside builds, so I assume you ran a build too. Checking it out now.

mischnic · 2019-06-10T20:32:51Z

so I assume you ran a build

Yes

diff --git a/packages/examples/simple/package.json b/packages/examples/simple/package.json
index 77ccef86..6c15a4ea 100644
--- a/packages/examples/simple/package.json
+++ b/packages/examples/simple/package.json
@@ -4,7 +4,7 @@
   "license": "MIT",
   "private": true,
   "scripts": {
-    "demo": "PARCEL_WORKERS=0 parcel2 src/index.js",
+    "demo": "PARCEL_WORKERS=0 parcel2 build src/index.ts",
     "clean-demo": "rm -rf .parcel-cache dist && yarn demo"
   },
   "devDependencies": {
diff --git a/packages/examples/simple/src/foo.js b/packages/examples/simple/src/foo.ts
similarity index 100%
rename from packages/examples/simple/src/foo.js
rename to packages/examples/simple/src/foo.ts
diff --git a/packages/examples/simple/src/index.js b/packages/examples/simple/src/index.ts
similarity index 100%
rename from packages/examples/simple/src/index.js
rename to packages/examples/simple/src/index.ts

devongovett · 2019-06-16T03:59:55Z

packages/configs/tsc/index.json

@@ -0,0 +1,9 @@
+{


Not sure if we need a whole config for this. It's overriding one transformer, and a user would need their own .parcelrc to enable it anyway, so it wouldn't be hard for them to do it themselves.

devongovett · 2019-06-16T04:00:21Z

packages/configs/tsc/index.json

+  "transforms": {
+    "*.{ts,tsx}": [
+      "@parcel/transformer-typescript-tsc",
+      "@parcel/transformer-js"


This shouldn't be needed as part of the .ts config (and in fact will never run). The tsc transformer will output an asset of type js, which will cause it to jump to another pipeline and never run the rest of the ts pipeline.

devongovett · 2019-06-16T04:02:09Z

packages/configs/default/index.json

@@ -1,7 +1,7 @@
 {
  "bundler": "@parcel/bundler-default",
  "transforms": {
-    "*.{js,mjs,jsm,jsx,es6}": [
+    "*.{js,mjs,jsm,jsx,es6,ts,tsx}": [


I think typescript should be a separate transformer plugin, even if it uses babel internally. We'll be able to share ASTs anyway, so shouldn't be a perf hit.

devongovett · 2019-06-16T04:07:26Z

packages/transformers/babel/src/config.js

+        hasPlugin(babelrc.config.plugins, [
+          '@babel/transform-typescript',
+          '@babel/plugin-transform-typescript'
+        ]));


I don't really think we should be doing this kind of searching for babel plugins and manipulation of user config anymore. It's not safe, and not possible to do fully correctly especially if we are supporting babel.config.js, but also if someone used e.g. a forked version of the typescript preset or something. I think we should just pull this into a separate parcel transformer to just run babel-preset-typescript. If it happens to run again as part of the babel transformer later on, it shouldn't break anything (ts is already gone).

I was also thinking of splitting the babel-preset-env and flow steps into their own parcel plugins too, and having the babel transformer only run if there is an explicit .babelrc config... not sure yet.

I don't really think we should be doing this kind of searching for babel plugins and manipulation of user config anymore

I agree with this, Jest does it and I've had to debug that behavior in Jest several times and it's been a complete nightmare that takes several days to work out every time.

devongovett · 2019-06-16T04:10:34Z

packages/transformers/typescript-tsc/src/TSCTransformer.js

+    let typescript =
+      config == null
+        ? // $FlowFixMe
+          require('typescript')


hmmmm... should we always require typescript be installed locally?

wbinnssmith · 2019-07-11T05:27:19Z

This is good for another review in order to unblock @DeMoorJasper's work on diagnostics.

As discussed with @padmaia and @devongovett , we're going to leave the inferring of and mutating babel config in place for now. We'll split up the babel transform and not modify the user's babel configuration going forward.

devongovett

Looks good. Merging this for now to unblock Jasper. It will become part of the babel preset we offer by default as discussed.

wbinnssmith added the ✨ Parcel 2 label May 24, 2019

wbinnssmith requested review from DeMoorJasper, devongovett and padmaia May 24, 2019 20:24

wbinnssmith force-pushed the wbinnssmith/tsc branch 2 times, most recently from 0b7416d to 5d83b02 Compare May 24, 2019 20:39

wbinnssmith commented May 24, 2019

View reviewed changes

devongovett reviewed May 25, 2019

View reviewed changes

wbinnssmith force-pushed the wbinnssmith/tsc branch from 5d83b02 to b84af3f Compare June 1, 2019 22:18

mischnic reviewed Jun 2, 2019

View reviewed changes

wbinnssmith force-pushed the wbinnssmith/tsc branch 2 times, most recently from f2390ff to 52a2b61 Compare June 3, 2019 23:23

wbinnssmith force-pushed the wbinnssmith/tsc branch 2 times, most recently from d498a56 to 510277d Compare June 3, 2019 23:33

mischnic mentioned this pull request Jun 8, 2019

Better TypeScript support #1378

Closed

DeMoorJasper changed the title ~~Typescript transform support through Babel (default) and tsc~~ V2: Typescript transform support through Babel (default) and tsc Jun 9, 2019

wbinnssmith force-pushed the wbinnssmith/tsc branch from 510277d to 3203d69 Compare June 10, 2019 20:29

devongovett reviewed Jun 16, 2019

View reviewed changes

DeMoorJasper and others added 6 commits July 10, 2019 15:25

ts transformer

c548147

Rename to typescript-tsc transformer

7c0d611

Update typescript

e6d6490

Add tsc config and pass some tests

f0b59c2

Add Babel typescript support

702b80f

Support user's typescript

7e1aa50

wbinnssmith force-pushed the wbinnssmith/tsc branch 2 times, most recently from 5428f04 to 8ab6497 Compare July 10, 2019 22:32

Always use user's typescript

f95db5c

wbinnssmith force-pushed the wbinnssmith/tsc branch from 8ab6497 to f95db5c Compare July 11, 2019 05:04

Add typescript defs from flow-typed

dc7ad4e

Remove custom config

966361f

wbinnssmith force-pushed the wbinnssmith/tsc branch from 830fe64 to 966361f Compare July 11, 2019 06:19

devongovett approved these changes Jul 13, 2019

View reviewed changes

devongovett merged commit a547688 into v2 Jul 13, 2019

devongovett deleted the wbinnssmith/tsc branch July 13, 2019 03:20

stoically mentioned this pull request Nov 10, 2019

TypeScript stoically/temporary-containers#322

Merged

3 tasks

bbugh mentioned this pull request Jan 20, 2020

Parcel 2: TypeScript automatic zero configuration not type checking or reading tsconfig.json #4022

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

V2: Typescript transform support through Babel (default) and tsc #3083

V2: Typescript transform support through Babel (default) and tsc #3083

wbinnssmith commented May 24, 2019 •

edited

Loading

wbinnssmith May 24, 2019

padmaia May 24, 2019 •

edited

Loading

wbinnssmith May 24, 2019 •

edited

Loading

wbinnssmith May 24, 2019

padmaia May 24, 2019

devongovett commented May 25, 2019

devongovett May 25, 2019

DeMoorJasper commented May 28, 2019 •

edited

Loading

wbinnssmith commented Jun 1, 2019

mischnic Jun 2, 2019 •

edited

Loading

wbinnssmith commented Jun 3, 2019

mischnic commented Jun 4, 2019

wbinnssmith commented Jun 4, 2019 •

edited

Loading

mischnic commented Jun 4, 2019

dgreensp commented Jun 7, 2019

mischnic commented Jun 8, 2019

wbinnssmith commented Jun 10, 2019

wbinnssmith commented Jun 10, 2019

mischnic commented Jun 10, 2019

devongovett Jun 16, 2019

devongovett Jun 16, 2019

devongovett Jun 16, 2019

devongovett Jun 16, 2019

devongovett Jun 16, 2019

jamiekyle-eb Jul 3, 2019

devongovett Jun 16, 2019

wbinnssmith commented Jul 11, 2019

devongovett left a comment

V2: Typescript transform support through Babel (default) and tsc #3083

V2: Typescript transform support through Babel (default) and tsc #3083

Conversation

wbinnssmith commented May 24, 2019 • edited Loading

Choose a reason for hiding this comment

padmaia May 24, 2019 • edited Loading

Choose a reason for hiding this comment

wbinnssmith May 24, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devongovett commented May 25, 2019

Choose a reason for hiding this comment

DeMoorJasper commented May 28, 2019 • edited Loading

wbinnssmith commented Jun 1, 2019

mischnic Jun 2, 2019 • edited Loading

Choose a reason for hiding this comment

wbinnssmith commented Jun 3, 2019

mischnic commented Jun 4, 2019

wbinnssmith commented Jun 4, 2019 • edited Loading

mischnic commented Jun 4, 2019

dgreensp commented Jun 7, 2019

mischnic commented Jun 8, 2019

wbinnssmith commented Jun 10, 2019

wbinnssmith commented Jun 10, 2019

mischnic commented Jun 10, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wbinnssmith commented Jul 11, 2019

devongovett left a comment

Choose a reason for hiding this comment

wbinnssmith commented May 24, 2019 •

edited

Loading

padmaia May 24, 2019 •

edited

Loading

wbinnssmith May 24, 2019 •

edited

Loading

DeMoorJasper commented May 28, 2019 •

edited

Loading

mischnic Jun 2, 2019 •

edited

Loading

wbinnssmith commented Jun 4, 2019 •

edited

Loading