refactor(Build) Add typescript compiler

[michaelw85]

Refactor project to run from dist folder and add typescript transpiler.

Related to #179

[3cp]

Please hold TS for few more days :-) There is one big refactoring PR coming soon (in hrs).

[michaelw85]

@huochunpeng Ok, there is no rush. I did make the build pass so I it is ready for review. I do not expect allot of merge conflicts.

What this PR does is add a build setup with a Typescript compiler and copies resources + non js/ts files to the dist folder. I tried to touch a minimum amount of existing code, I did change 2 files to TS extension to prove the setup works and it also enables the first bare minimum for generating d.ts files.

After this is merged I/we can proceed with the next step which is getting TS support setup for jasmine and after that files can be converted one by one.

[3cp]

I am touching lots of release-check files in #1075. Please hold this one bit longer.

[michaelw85]

I am touching lots of release-check files in #1075. Please hold this one bit longer.

NP I was just trying to resolve merge conflict. I also want to add support for unit tests before getting this merged. I will update this to WIP

[michaelw85]

@huochunpeng now that #1075 is merged would it be possible to get this reviewed and possible even merged?

[3cp]

It is going to be reviewed by other team members as I am not a TS user.

Also we are preparing final v1 release. This is probably safer to leave TS refactoring to v1.1. cc @EisenbergEffect

[veikkoeeva]

@michaelw85 I'm a n00b but would like to change the TS setup, so asking a bit about the appoach. :)

I took the CLI output and modified it a bit, see at https://github.com/veikkoeeva/MultioutputWebpack (it should work if you remove the webpack-babel-multi-target-plugin from the pipeline). The idea wasn't to transpile to es5, but the highest possible and then use Babel to transpile down. For performanc etc. use the usual methods of forking and that (the demo is so little it doesn't seem to matter so didn't go through it). So it appears the difference in our approach comes to use of Babel at least.

I did first "the usual" of using Babel 7 to just strip away types, but ran into trouble with decorators. The problem basically boils down into decorator metadata and is explained at https://discourse.aurelia.io/t/typescript-babel-7/2381/6 (see towards the end of the short thread).

After that I thought that maybe a better approach would be to use TS as usually, transpile to highest JS and then use Babel to build a es5 or some other package. Basically to set up differential serving. It looks a bit involved with setting one or more entries, so I searched solutions and spotted an excellent looking plugin webpack-babel-multi-target-plugin, but ran into a problem, see at DanielSchaffer/webpack-babel-multi-target-plugin#28. It's probably related to how Aurelia pipeline works, but I do also have very new Babel 7.4 with the brand new core-js 3.0 set up too.

As a side note, I removed the direct link to core-js in the Aurelia source code and let BabylonJS to load and it dropped the size of the (dev) bundle to 1/3. I also moved Webpack to TS in one branch, but I'm not sure if this is something anyone would want. :D I'm not also sure if source maps will cause problem once Babel transpilation works (differential serving or not) and how would one solve it. Originally https://twitter.com/EisenbergEffect/status/1120542146237534211 prompoted me to come to here, the note about putting money into the pot (related to the Twitter discussion) refers to differential serving, but I suppose this is the larger context for me.

[michaelw85]

@veikkoeeva Thanks for your input!
I do not consider myself an expert on TS and builds so any input is appreciated. My goal with this PR was to get something going. A bare minimum with the least amount of impact allowing future conversion to TS and creation of dts files.

I currently do not have time to completely go over your comment and examples but I did want to reply. I hope to be able to have a look later this week.

If you want you can fork my branch, adapt it and create a new improved PR.

[veikkoeeva]

@michaelw85 It's OK, I don't want to imply you need to do anything. You did go further in making something useful to others already. If this is useful, all the better. Maybe gives thoughts to someone.

[michaelw85]

@3cp This PR is going stale and it would be a waste to delete it. Any update when this could be reviewed & merged?

[3cp]

We said we are going to delay TS conversion to post v1 release. Because we are very close to v1 release.

Since I am not a TS user, probably @bigopon will help to review this change. I can ensure you we are not ignoring this contribution:)

[michaelw85]

Hearing that it's not ignored is reassuring.

How can I track any progress on CLI releases? I feel like I'm a bit in the dark when it comes to any releases related to Aurelia in general. I have no idea where to go to get a overview.

[EisenbergEffect]

@michaelw85 Just wanted to let you know we aren't ignoring you. We're dealing with a TS-related issue in the DI library. Once we get that all sorted out, we'll circle back to this. Just didn't want you wondering what was going on.

[michaelw85]

@EisenbergEffect Thank you for the explanation I was actually at a point where I started to think to close this PR and delete the changes.

[fkleuver]

Just my 2c, it looks fine to me. The thing with the package.json npm script is just a nitpick as it has no functional impact, it's just a verbose alias like npm install instead of npm i.

[fkleuver]

@EisenbergEffect @3cp @bigopon Let's get this in?

[EisenbergEffect]

@michaelw85 Thanks for working on this! I wanted to share that for Aurelia vNext, we are planning to take a slightly different approach. Please have a read through of the RFC here: aurelia/aurelia#509 This might be a good opportunity for you to get more deeply involved with Aurelia and vNext.

[michaelw85]

@EisenbergEffect thx for the info. Any particular area you think I could/should be involved?

[EisenbergEffect]

@michaelw85 If you are interested in tooling, there's opportunities to work on the cli helpers, the convention system, AoT, VS Code integration, and debug plugin. Have a read through the issue above and poke around the vnext repo a bit and let me know what seems interesting to you. We'll get you hooked in.

[michaelw85]

Any of those sounds interesting, I guess AOT would be the most challenging. I've done some work on vscode plugins in the past, really enjoyed that.

I will have a look at the vnext repo asap

[fkleuver]

@michaelw85 I am working on AOT as we speak, so if you want to help out with integrating it into VS Code that would be perfect. What did you work on previously? Autocomplete etc?

[3cp]

I am reviewing the build.

@michaelw85 why we need this to make the final npm package full of duplicated skeleton files. Those skeleton files are static resources, I see no reason to duplicate them.

If need to access the skeleton files, might need to change the code that reads those static resources from aurelia-cli pacakge root, instead of from the relative location of the code dist/lib/...

"build:copy-files": "copyfiles -e ./skeleton/**/*.{js,ts} -e ./lib/**/*.{js,ts} ./lib/**/* ./skeleton/**/* ./dist",

Also I have to add

"pretest": "npm run build && npm run lint",
    "prepare": "npm run build",

To ensure build before test and publish.

Another thing I am doing is to switch from Yarn to NPM in release-check to probably handle aurelia/cli#master dep prepare scripts for the build. Yarn has bug yet to be fixed in this use case.

[3cp]

And somehow the json file is not copied over to dist/.

⋊> ~/playground au new some
Global aurelia-cli v1.0.2
Invalid Command: new
                      _ _          ____ _     ___
  __ _ _   _ _ __ ___| (_) __ _   / ___| |   |_ _|
 / _` | | | | '__/ _ \ | |/ _` | | |   | |    | |
| (_| | |_| | | |  __/ | | (_| | | |___| |___ | |
 \__,_|\__,_|_|  \___|_|_|\__,_|  \____|_____|___|

Error: Cannot find module '../new/command.json'
Require stack:
- /Users/huocp/aurelia/cli/dist/lib/commands/help/command.js
- /Users/huocp/aurelia/cli/dist/lib/cli.js
- /Users/huocp/aurelia/cli/dist/lib/index.js
- /Users/huocp/aurelia/cli/dist/bin/aurelia-cli.js
    at Function.Module._resolveFilename (internal/modules/cjs/l

[3cp]

The reason that unit tests didn't catch this, is probably due to "jasmine-ts" works directly on lib/ files, not the compiled dist/ files.

[3cp]

Given the complications around compilation.

I am not TS user, but is this an approachable solution to let typescript to load at runtime for every lib/**/*.ts file? Like the way we support ts gulp task in final app project.

Runtime transpiling can remove all the compilation steps and file duplications. But some TS user might have better understand of this topic.

This is how we handled ts gulp task.

function installTypeScript() {
  let ts = require('typescript');

  let json = require.extensions['.json'];
  delete require.extensions['.json'];

  require.extensions['.ts'] = function(module, filename) {
    let source = fs.readFileSync(filename);
    let result = ts.transpile(source, {
      module: ts.ModuleKind.CommonJS,
      declaration: false,
      noImplicitAny: false,
      noResolve: true,
      removeComments: true,
      noLib: false,
      emitDecoratorMetadata: true,
      experimentalDecorators: true
    });

    return module._compile(result, filename);
  };

  require.extensions['.json'] = json;
}

[michaelw85]

@fkleuver

@michaelw85 I am working on AOT as we speak, so if you want to help out with integrating it into VS Code that would be perfect. What did you work on previously? Autocomplete etc?

Yes I've created my own autocomplete plugin at my previous employer to help write e2e tests using the custom layer I created on top of wdio. I also contributes to a plugin that provided jump to code for js code using with require.

[michaelw85]

@3cp

@michaelw85 why we need this to make the final npm package full of duplicated skeleton files. Those skeleton files are static resources, I see no reason to duplicate them.

Because normally you only package dist, there is no need to ship the source with your package.

And somehow the json file is not copied over to dist/.

I will have to tweak the copy command a bit, should be an easy fix.

The reason that unit tests didn't catch this, is probably due to "jasmine-ts" works directly on lib/ files, not the compiled dist/ files.

Yes it will compile on the fly, this uses the same configuration as the build process so in terms of transpiling it is identical. Loading files is done relative so yes this would work in a test but fail if the build command does not copy a json file.

I am not TS user, but is this an approachable solution to let typescript to load at runtime

Using node-ts that could work although it gives unnecessary overhead in my opinion. I made a mistake in the copy files task when introducing a completely new setup. I don't think we should blow this out of proportions. Maybe I could create a post build task that checks the total amount of files, compares src with dist. I think they should be equal in amount, this should be an easy script to verify the copy task is configured properly.

[3cp]

skeleton Folder is NOT source code to be compiled. Please don’t copy them, there is no point.

Set “files” field in package.json to include bin dist skeleton folders. That should be enough.

[3cp]

If you skip compiling bin folder, only compile lib to dist instead of dist/lib, then you wouldn’t have issue on relative path to skeleton folder.

[michaelw85]

If you skip compiling bin folder, only compile lib to dist instead of dist/lib, then you wouldn’t have issue on relative path to skeleton folder.

I could do that, if that's what you prefer.

I think it's weird for anything inside dist to require something outside of the folder. I always build my packages so that dist is what is shipped, anything outside of dist will not be published. This gives you full control over what you want or do not want to publish as a package. This makes the package only contain the files you need. If all packages would do this npm install would be allot quicker.

[3cp]

The fact is there is always things outside dist folder published, including package.json which can load from the code inside dist.

[3cp]

Many libs load their own package.json in js code to print out version, read config like jest.

[3cp]

BTW, in the following up PR, please run through release-check locally to identify potential problems. This change touched many files, it might have few more small complications. There are few adjustments you need on the release-check implementation, see my previous comments. Thx!

In release check, you need to target your branch head instead of aurelia/cli#master.

[3cp]

@michaelw85 just check with you, what's the roughly expected time frame for the fix?

[michaelw85]

Sorry for the delay, I tried to start multiple times but other things came up. Bit short on time after work. I might be able to have a look tonight. If a proper fix is not possible I will at least send a pr to include the missing file.

[3cp]

That's fine. Take your time to have a proper fix, we will merge if the whole release-check passes.