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

Implement an official TypeScript compiler plugin. #10610

Merged
merged 2 commits into from Jul 10, 2019

Conversation

benjamn
Copy link
Contributor

@benjamn benjamn commented Jul 7, 2019

As requested in meteor/meteor-feature-requests#285.

This plugin package enables the use of TypeScript modules with .ts or .tsx file extensions in Meteor applications and packages, alongside .js modules (or whatever other types of modules you might be using).

Usage

The typescript package registers a compiler plugin that transpiles TypeScript to plain ECMAScript, which is then compiled by Babel for multiple targets (server, modern browsers, legacy browsers, cordova). By default, the typescript package is included in the .meteor/packages file of all new Meteor applications.

To add the typescript package to an existing app, run the following command from your app directory:

meteor add typescript

To add the typescript package to an existing package, include the statement api.use('typescript'); in the Package.onUse callback in your package.js file:

Package.onUse(function (api) {
  api.use('typescript');
});

Supported TypeScript features

Almost all TypeScript syntax is supported, though this plugin currently does not attempt to provide type checking (just compilation).

Since the Meteor typescript package runs the official TypeScript compiler before running Babel, it does not suffer from the same caveats known to affect the Babel TypeScript transform. In particular, namespaces are fully supported.

However, as of this writing, the Meteor typescript package compiles TypeScript modules individually, using the transpileModule function, which means that certain cross-file analysis and compilation will not work. For example, export const enum {...} is not fully supported, though const enum {...} works when confined to a single module.

Unlike the Babel implementation of TypeScript, there is nothing fundamentally preventing Meteor from compiling all TypeScript modules together, rather than individually, which would enable full support for features like export const enum. That's an area for future improvement, though we will have to weigh the performance implications carefully.

As of this writing, tsconfig.json files are ignored by the plugin. There's nothing fundamentally preventing the Meteor typescript plugin from accepting configuration options from tsconfig.json, but for now we've kept things simple by avoiding configuration complexities. You may still want to have a tsconfig.json file in your application root directory to configure the behavior of editors like VSCode, but it will not be respected by Meteor.

Finally, since the TypeScript compiler runs first, syntax that is not understood by the TypeScript compiler, such as experimental ECMAScript proposals, may cause the TypeScript parser to reject your code. You can use .babelrc files to configure Babel compilation, but TypeScript still
has to accept the code first.

Areas for improvement

  • Compile all TypeScript modules at the same time, rather than compiling them individually, to enable cross-module analysis and compilation. In the meantime, if you need this behavior, consider using adornis:typescript.

  • Use the version of typescript installed in the application node_modules directory, rather than the one bundled with meteor-babel. We will attempt to keep meteor-babel's version of typescript up-to-date, but it would be better to leave this decision to the application developer.

  • Generate .d.ts declaration files that can be consumed by external tools. In particular, a Meteor package that uses TypeScript could generate .d.ts files in the /node_modules/meteor/package-name/ directory, which would allow tools like VSCode to find the right types for the package when importing from meteor/package-name.

  • Cache the output of the TypeScript compiler separately from the output of Meteor's Babel compiler pipeline. The TypeScript compiler does not compile code differently for different targets (server, modern, legacy, etc.), so its results could theoretically be reused for all targets.

  • Make the typescript plugin look up tsconfig.json files (similar to how babel-compiler looks up .babelrc files) and obey (some of) the configuration options.

@benjamn benjamn added this to the Release 1.8.2 milestone Jul 7, 2019
@benjamn benjamn self-assigned this Jul 7, 2019
}, function () {
return new TypeScriptCompiler({
react: true,
typescript: true,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See meteor/babel#25 for details about how this is implemented in meteor-babel.

@yorrd
Copy link

yorrd commented Jul 8, 2019

@benjamn glad you're working on this 😍 . I see you decided on implementing your own caching. At adornis, we were evaluating whether to implement support for incremental builds from typescript itself. Do you think that could have performance benefits over caching on a file by file basis? We've been using our predecessors caching and rebuild times get really long if a lot of files import that specific file which they don't (at least not in that extent) when using tsc. Haven't investigated this further, just random thoughts...

EDIT: thanks for the shoutout :)

@benjamn benjamn merged commit c1cd8fc into release-1.8.2 Jul 10, 2019
@benjamn benjamn deleted the core-typescript-plugin branch July 15, 2019 14:37
@yorrd
Copy link

yorrd commented Jul 16, 2019

@benjamn how do you do absolute imports? 'imports/client/my-component' results in module not found. The exact same code worked with our old plugin. We have like 200 absolute imports, please don't make us change them :D

@gerwinbrunner
Copy link
Contributor

@benjamn @yorrd
I have the same issue with the imports. Did you find a solution?

@benjamn
Copy link
Contributor Author

benjamn commented Jul 26, 2019

The nice thing about using Babel for part of the compilation pipeline is that we can use a Babel plugin to rewrite imported module specifiers.

I think we can automate this better, but here's a custom plugin that I wrote for one of our internal application development teams:

const fs = require('fs');
const { dirname, join } = require('path');

const appDir = dirname(dirname(__dirname));
const hasOwn = Object.prototype.hasOwnProperty;

// These directory names are computed only once per build process, so you
// may need to restart Meteor to pick up any changes.

const nodeModulesDirNames = Object.create(null);
fs.readdirSync(join(appDir, 'node_modules')).forEach((dir) => {
  if (!dir.startsWith('.')) {
    nodeModulesDirNames[dir] = true;
  }
});

const topLevelDirNames = Object.create(null);
fs.readdirSync(appDir).forEach((item) => {
  if (!item.startsWith('.') && item !== 'node_modules') {
    const stat = fs.statSync(item);
    if (stat.isDirectory()) {
      topLevelDirNames[item] = stat;
    }
  }
});

// Babel plugin that rewrites import declarations like
//
//   import foo from "imports/bar"
//
// to use properly absolute identifier strings like
//
//   import foo from "/imports/bar"
//
// TypeScript can understand imports/bar thanks to the "paths" property in
// tsconfig.json, but Node and Meteor treat imports/bar as referring to a
// package in node_modules called "imports", which does not exist.
//
// If a directory name exists in both node_modules and the root
// application directory, the node_modules package will take precedence.

module.exports = function plugin(api) {
  function helper(path) {
    // An ImportDeclaration will always have a source, but an
    // ExportAllDeclaration or ExportNamedDeclaration may not.
    const { source } = path.node;
    if (!source) return;

    const sourceId = source.value;
    const name = sourceId.split('/', 1)[0];

    // If the first component of the sourceId is a top-level directory in
    // the application, and not the name of a directory in node_modules,
    // prepend a leading / to make it an absolute identifier that Meteor's
    // module system can understand.
    if (
      hasOwn.call(topLevelDirNames, name) &&
      !hasOwn.call(nodeModulesDirNames, name)
    ) {
      path.get('source').replaceWith(api.types.stringLiteral(`/${sourceId}`));
    }
  }

  return {
    name: 'transform-non-relative-imports',
    visitor: {
      ImportDeclaration: helper,
      ExportAllDeclaration: helper,
      ExportNamedDeclaration: helper,
    },
  };
};

Until we implement this officially, you can put this code in a local file (one that is not eagerly loaded by Meteor), and then refer to it in the plugins section of a .babelrc file.

@yorrd
Copy link

yorrd commented Aug 5, 2019

@benjamn @gerwinbrunner I must be missing something. I included the file in the babelrc (which is otherwise empty) and it seems like the paths are being replaced correctly. However, there are still exceptions failing to import imports/..., so those changed paths somehow don't make it to the compiler. Of course, I tried resetting and stuff,

This is my package file, am I missing something?

meteor-base@1.4.0             # Packages every Meteor app needs to have
mobile-experience@1.0.5       # Packages for a great mobile UX
mongo@1.8.0-alpha190.9                   # The database Meteor supports right now
reactive-var@1.0.11            # Reactive variable for tracker
tracker@1.2.0                 # Meteor's client-side reactive programming library

standard-minifier-css@1.5.3
standard-minifier-js@2.4.1

dynamic-import@0.5.1
server-render@0.3.1
ecmascript
es5-shim@4.8.0
jagi:astronomy
static-html
accounts-password@1.5.1
ostrio:files
email@1.2.3
alanning:roles
jagi:astronomy-timestamp-behavior
fourseven:scss@4.9.0
nimble:restivus
okgrow:analytics
raix:push
promise@0.11.2
babel-runtime@1.4.0-beta182.17
typescript

@coagmano
Copy link
Contributor

coagmano commented Aug 5, 2019

Does it help to specify the root path in the tsconfig?
Something like this (untested):

{
  "compilerOptions" : {
    "baseUrl": "./",
    "paths" : {
      "*": ["./*"]
    }
  }
}

More info on TS module resolution here: microsoft/TypeScript#5039

@yorrd
Copy link

yorrd commented Aug 6, 2019

@coagmano thanks for the idea! Already have those options though

@andrei-markeev
Copy link

@benjamn, regarding react: true, did I understand correctly that it isn't then possible to use babel-plugin-transform-vue-jsx as they are mutually exclusive with preset-react?

Would be great to have an option to use Vue+jsx with the official typescript plugin...

@yorrd
Copy link

yorrd commented Sep 18, 2019

Just wondering, this is probably not using TS 3.6's incremental compile API? Would probably have a good performance impact. Any plans on implementing? Are pull requests welcome for this sort of thing?

Btw: Sorry never reported back, I just had no idea on how to make a babel plugin (you need a package.json with a main section). Working flawlessly so far.

@cvolant
Copy link

cvolant commented Sep 19, 2019

Conflict: Constraint errors

I tried to meteor add typescript to move my app to typescript, but I struggled a long time with constraints errors without getting any success...
To make it really short:

  • I tried to meteor create to see how the typescript package is newly included in the .meteor/packages file, but it isn't yet...
  • Then I tried to meteor add typescript in a brand new --bare created app, but I keep getting constraints errors for packages "modules", "babel-runtime", and "babel-compiler".

Here are the conflicts prompted for the last two (those for modules are far longer):

Conflict: Constraint babel-runtime@1.5.0-alpha190.10 is not satisfied by babel-runtime 1.3.0.
Constraints on package "babel-runtime":
* babel-runtime@~1.3.0 <- top level
* babel-runtime@1.3.0 <- ecmascript 0.12.7
* babel-runtime@1.5.0-alpha190.10 <- typescript 3.7.0-alpha190.10

Conflict: Constraint babel-compiler@7.5.0-alpha190.10 is not satisfied by babel-compiler 7.3.4.
Constraints on package "babel-compiler":
* babel-compiler@~7.3.4 <- top level
* babel-compiler@7.3.4 <- ecmascript 0.12.7
* babel-compiler@7.3.4 <- minifier-js 2.4.1 <- standard-minifier-js 2.4.1
* babel-compiler@7.3.4 <- standard-minifier-js 2.4.1
* babel-compiler@7.5.0-alpha190.10 <- typescript 3.7.0-alpha190.10

I checked my global npm packages with npm list -g --depth=0 but babel doesn't appear there, neither does modules.
Probably I am doing something wrong since it doesn't seem to bother anyone else, but I can't find what... Any clue?

@yorrd
Copy link

yorrd commented Sep 19, 2019

@cvolant do you run on 1.8.2-beta.*?

@benjamn just noticed: dynamic imports are not possible with absolute paths. Should the babel plugin from above solve that?

@benjamn
Copy link
Contributor Author

benjamn commented Sep 19, 2019

regarding react: true, did I understand correctly that it isn't then possible to use babel-plugin-transform-vue-jsx as they are mutually exclusive with preset-react?

@andrei-markeev If you put babel-plugin-transform-vue-jsx in your .babelrc file (in the plugins section), I believe it will win out over the default React-oriented JSX handling that ships with Meteor. Happy to investigate if you discover that's not the case.

Just wondering, this is probably not using TS 3.6's incremental compile API? Would probably have a good performance impact. Any plans on implementing? Are pull requests welcome for this sort of thing?

@yorrd I am definitely open to moving the TypeScript-to-ES2019 compilation (and type checking) into the typescript plugin (here), rather than meteor-babel (here), since the plugin sees all TS files at once, whereas the meteor-babel implementation only sees one module at a time.

@benjamn
Copy link
Contributor Author

benjamn commented Sep 19, 2019

With the release of Meteor 1.8.2-rc.0 this morning, there's now a TypeScript skeleton app that you can create with the following command:

meteor create --release 1.8.2-rc.0 --typescript new-typescript-app

It's based on the meteor create --react app (so it uses React), and it includes a recommended tsconfig.json file.

I'm pretty happy with the absence of TypeScript errors and warnings in this project, though that depends partly on the use of @types/meteor, which seems to be somewhat out of date. Hopefully the use of @types/meteor in this official TypeScript skeleton app will provide some encouragement for contributors to that project to continue the great work they've done so far.

One of the reasons I think the tsconfig.json file is important is that it demonstrates adding a "paths": { "/*": ["*"] } rule to support the import ... from "/imports/some/module" style. This wasn't always possible, because the TypeScript team dragged their feet on allowing "absolute" (leading /) paths entries, but it seems to work in typescript@3.6.3. In other words, I don't think we should be pretending that imports/some/module (nonstandard, unsupported) is synonymous with /imports/some/module (working since Meteor 1.3). Using a leading / is the way to go.

@cvolant
Copy link

cvolant commented Sep 20, 2019

do you run on 1.8.2-beta.*?

... So that is what I was doing wrong. Thank you!

there's now a TypeScript skeleton app that you can create with the following command

Tried it! Awesome! :)

@copleykj
Copy link
Contributor

Hopefully the use of @types/meteor in this official TypeScript skeleton app will provide some encouragement for contributors to that project to continue the great work they've done so far.

@benjamn Can you elaborate on some issues that exist with the current type definitions in @types/meteor? I'm hoping to spur some community work on this.

@yorrd
Copy link

yorrd commented Sep 21, 2019

@benjamn we're celebrating this a lot in the office. Thanks.

About the tsconfig and your remark of moving the typescript precompilation into its package: what's the current state of affairs on this? Are you working on this? Would it speed up the process if we helped with a pull request? We're excited for that change because then we can 1) load the tsconfig (I don't think it's respected atm?) and 2) fork the package in case we want to test something and propose a pull request more easily.

Greetings!

@nicu-chiciuc
Copy link
Contributor

Regarding @types/meteor. Would it make sense to focus on transforming the current meteor core packages to TS so that the definitions are implicit and there is no need to rely on @types/meteor?

@benjamn
Copy link
Contributor Author

benjamn commented Sep 22, 2019

@nicu-chiciuc I absolutely agree with that, and it's worth noting that we control the meteor npm package name, so Meteor could potentially output files like node_modules/meteor/package-name.d.ts, based on the compiled TypeScript source code of the package.

@MastroLindus
Copy link

MastroLindus commented Oct 1, 2019

Is there a chance that we are going to be able to use the typescript version installed in the node_modules folder anytime soon?
For me that and the fact that tsconfig.json is ignored are the only two things stopping me to start adopting this plugin...
Thanks for the fantastic work so far @benjamn

@perbergland
Copy link
Contributor

When I tested this by upgrading my project to 1.8.2, removing adornis:typescript and adding "typescript", I couldn't see any type errors in the console when I deliberately broke a thing or two.

That is a vital part of my current workflow (to see what things break when meteor hot-compiles changes to files and their dependencies) so I wonder if I was doing something wrong or if this is currently by design.

@benjamn
Copy link
Contributor Author

benjamn commented Nov 15, 2019

@perbergland That's by design. The new typescript package is deliberately focused on just compiling the code, so you should rely on an editor like VSCode (or npx tsc --noEmit) to report errors for the time being. This is an area where we can easily make improvements in the future, but I didn't want Meteor-reported TypeScript errors to be duplicative with errors reported by other tools, in this first version of the typescript package.

@MastroLindus We're following the precedent set by the ecmascript and babel-compiler packages, where you don't have to install Babel yourself, because it's managed by the Meteor packages. This abstraction has been essential to making major updates (from Babel 5 to 6 to 7) completely behind the scenes. If we let developers use whatever version of Babel they'd installed in node_modules, we would have no ability to update those dependencies safely when updating Meteor's Babel integration. I'm open to being persuaded that TypeScript is somehow different, but that's the current reasoning.

Also, I worry about leading people to think any set of options in tsconfig.json is valid and meaningful within Meteor. For example, there are several compilerOptions related to the output directory where TypeScript generates code, and Meteor's compiler plugin system does not dump generated code into a separate directory, so those tsconfig.json options are meaningless within Meteor. As another example, TypeScript has no concept of the modern/legacy differential bundling that Meteor 1.7 introduced, so it's important that Meteor is able to control the various compilation targets (server, modern browsers, and legacy browsers) that are used. I could write a whole blog post about Meteor's handling of import and export syntax, so the tsconfig.json options related to the module system are equally meaningless within Meteor. I hope I'm painting a picture of why we need the typescript abstraction to be relatively air-tight, and why I feel that pretending to support additional configuration would be a path to confusion. With that said, if there are particular tsconfig.json options that you think would be useful to support, please let us know, and we can decide together whether they make sense!

@perbergland
Copy link
Contributor

Thanks for the info, Ben.

For my use case I think I would like to still have TS output in the console and also to fail builds that don't pass when running in build pipelines (circleci, github actions etc) and give proper errors there.

TS is moving along very quickly so it seems essential to have the compiler support being able to release quickly after MS drops new versions. Therefore it's great that it's a separate package and not part of meteor core.

As for settings, I rely on quite a few to be able to work. The most important being strictNullChecks=true (YMMV) but I also need a bunch of the npm import flags because npm imports are so hard to get right in Typescript.

Below are my current settings for reference. I can't say I am certain I need all of them (and some are very poorly documented) but since my TS compile takes ~10min from scratch I don't often feel like experimenting when I get to a state that works :)

    "module": "commonjs",
    "target": "es5",
    "lib": ["es6","dom"],
    "moduleResolution": "node",
    "strictNullChecks": true,
    "experimentalDecorators": true,
    "emitDecoratorMetadata": true,
    "allowSyntheticDefaultImports": true,
    "sourceMap": true,
    "jsx": "react",
    "typeRoots": [
      "./node_modules/@types",
      "./imports/@types/global"
    ]

Another Very Important Attribute for build servers is to be able to cache TS results. For adornis:typescript, I cache the entire .meteor/local/.typescript-cache directory and this now keeps my bundle builds slightly below 10 minutes.

It would be a non-starter for me if it caching/restoring partial ts results is not possible.

My 10¢

@MastroLindus
Copy link

MastroLindus commented Nov 15, 2019

@benjamn I really appreciate your feedback on your decisions regarding the typescript configuration, they are really understandable.
Regarding the typescript in node_modules I don't have a strong argument with which I can really persuase you, aside from the fact that typescript is moving tremendously fast and be able to update it separately instead of having to wait for a meteor or plugin release would be very very valuable to me.
Also my configuration requires me to have typescript installed in the node_modules folder, and I would argue that it is a source of confusion to have a version installed but having another one doing the work behind the scenes.
Whoever I understand your need for consistency with the other packages, and it's not a real deal breaker for me (but definitely a very nice to have)

Regarding the configuration, my dealBreakes/must haves are:
-same as @perbergland : being able to see the output/errors in the console (relying on the editor or other external tools defies most of the point of having a type-safe code in the first place as you are not guaranteed to notice the problems before it's too late
-I rely (and would say require) on the following tsconfig settings:
strict: true,
experimentalDecorators: true,
noUnusedLocals: true,

"lib: [ "es2017", "dom" ],
"jsx": "react" (since you already handle react I don't think this is a problematic one)

I also use the exclude directive to exclude the folders .node_modules and .meteor
And finally I use the include folder to deal with some overrides of some types packages (mostly so that vscode can recognize them)

the full config is the following, but the stuff above are really what I do need (also some of it might be outdated or not needed anymore, but I touch it rarely these days)

{
  "compilerOptions": {
    "experimentalDecorators": true,
    "module": "commonjs",
    "target": "es6",
    "isolatedModules": false,
    "moduleResolution": "node",
    "allowJs": true,
    "strict": true,
    "noUnusedLocals": true,
    "strictNullChecks": true,
    "removeComments": false,
    "sourceMap": true,
    "lib": [
      "es2017",
      "dom"
    ],
    "jsx": "react"
  },
  "include": [
    "client/**/*",
    "server/**/*",
    "imports/**/*",
    "typesOverrides/**/*.d.ts"
  ],
  "exclude": [
    "node_modules",
    ".meteor"
  ]
}

@nicu-chiciuc
Copy link
Contributor

@MastroLindus regarding the option of viewing all the TS errors. We're using tsc --watch in a terminal window. Although it sometimes crashes when using node 8. Our setup is to basically use node 12 with tsc --watch and it seems to work fine. Having a CI step that checks if no-one forgot some TS error also helps.

@MastroLindus
Copy link

MastroLindus commented Nov 16, 2019

@nicu-chiciuc I used to have a similar approach (using tsc -watch to compile ts into js, and then let meteor pickup the js files) before I switched to adornis:typescript (that currently already provides me with compiler error output in the console, but soon will be deprecated in favor of the official package). Instead of running tsc --watch you could even just have it as a build task inside vs-code for what it is worth.

Honestly I would rather either compiling the typescript by myself with tsc, or having it in meteor, but having it both it's not a solution I like.
First, it might be easily possible that the ts version running in meteor and the one you use for tsc --watch can be different and be very dangerous, or at least source of bugs and/or confusion. You can have your tsc --watch telling you one thing while the meteor-compiled files can be very different.
That's also the reason why I am not a fan on relying on the IDE for errors, as also the editor can be configured with a different version (and also that editors like vscode only shows errors in opened files if you don't run a global build task)

Second I don't see the point in compiling the same files twice, especially since there's no reason or benefit doing it so.
I would be happy enough to be able to "opt-in" for seeing the errors through a meteor flag or something, (even if I still believe it should be the default behaviour), but until that changes (and the support for the ts flags I use) I will stick with adornis:typescript (but hoping that I can switch soon)

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

Successfully merging this pull request may close these issues.

None yet

10 participants