chore: add script to build typescript project references #1636

raymondfeng · 2018-08-22T20:12:17Z

Start to add support for TypeScript project references.

Checklist

npm test passes on your machine
New tests added or existing tests modified to cover all changes
Code conforms with the style guide
API Documentation in code was updated
Documentation in /docs/site was updated
Affected artifact templates in packages/cli were updated
Affected example projects in examples/* were updated

bajtos · 2018-08-23T15:09:49Z

bin/update-project-refs.js

+    rootRefs.push({
+      path: path.join(
+        path.relative(project.rootPath, pkgLocation),
+        'tsconfig.build.json',


Once we switch our monrepo to use project references, we should be able to get rid of tsconfig.build.json files and use regular per-package tsconfig.json file instead.

We are using tsconfig.build.json files to prevent VS Code from recognizing individual packages as individual TypeScript projects, and force it to treat our entire monorepo as a single TypeScript project. This is needed to make cross-package navigation and refactoring working.

With the new feature of project references, supported by VS Code too (now? in a near future?), there is no need to use a different configuration for VS Code and the compiler. We can rename per-package tsconfig.build.json files to regular tsconfig.json files.

We may be able to remove the repository-level tsconfig.json file too, but that requires changes in the way how tslint is executed. I am not sure if running tslint on per-package basis is something we actually want to do.

Ah, we cannot get rid of the top-level tsconfig.json file because this file must contain a list of references to all projects included in the monorepo.

We can probably remove tsconfig files from git and generate them as part of the build.

Sure, we can try that.

For now, I am fine with keeping the top-level tsconfig file in git. I think it may be easier for our users, as they don't have to run build before opening VS Code.

bin/update-project-refs.js

bajtos · 2018-08-23T15:14:07Z

I am looking forward to see this work finished and the pull request landed ❤️

raymondfeng · 2018-08-23T15:43:53Z

There are a few challenges to reach a working build:

We allow coexistence of dist8 and dist10 based on the Node version. tsc -b mode does not allow --outDir and --target command options. These values need to be set in tsconfig.json. We might have to dynamically create/update tsconfig.json for our build.
The tsc -b seems to have trouble handing our project layout:

metadata
  - src
  - test
  - index.ts
  - index.d.ts
  - index.js
  - dist10
    - index.d.ts
    - src
    - test

I'm playing with types in package.json to see if it helps. No luck so far.

bajtos · 2018-08-23T15:52:08Z

We allow coexistence of dist8 and dist10 based on the Node version. tsc -b mode does not allow --outDir and --target command options. These values need to be set in tsconfig.json. We might have to dynamically create/update tsconfig.json for our build.

Ouch.

I see why --outDir must be set in tsconfig.json file - there are multiple projects and each has a different output directory. However, could we perhaps work with the TypeScript team to allow --outDir with a project-relative path?

I don't see why --target is not allowed, but it's entirely possible I am missing something. Is this perhaps another opportunity for improving tsc?

The tsc -b seems to have trouble handing our project layout.

Note that package-level index.d.ts is exporting type definitions from dist8, but your example is using dist10 as the output directory. Could that be the source of the problem?

I guess the fix is to dynamically modify these package-level index.d.ts files depending on which version of Node.js we are running on.

Considering all these troubles and how little is Node.js 10.x adding in terms of JavaScript features, maybe we should simply abandon multi-target build and always transpile for Node.js 8.x only?

raymondfeng · 2018-08-23T15:57:11Z

I tried with both Node 8 and 10 to make sure dist8 is available, but the issue stays. You can try as follows:

Check out the branch and run npm i
Run ./bin/update-project-refs.js
cd packages/metadata
../build/bin/compile-package.js -b tsconfig.build.json or use tsc -b tsconfig.build.json

It complains about export * from './src' in testlab.

raymondfeng · 2018-08-24T17:05:19Z

Considering all these troubles and how little is Node.js 10.x adding in terms of JavaScript features, maybe we should simply abandon multi-target build and always transpile for Node.js 8.x only?

+1 if necessary.

bajtos · 2018-08-27T11:44:50Z

I'll try to run your changes on my machine later this week.

From what I remember, TypeScript in build mode uses .d.ts.map files to tell the compiler where to find the original sources (to go from the transpiled output back to .ts files). Our hand-writen package-level index file does not provide any map file. If we keep the current approach for selecting a dist directory based on Node.js version, then I am pretty sure we will need to roll out a hand-written index.d.ts.map file to accompany the hand-written index.d.ts file.

I don't know if the missing .d.ts.map file is the source of the error you are seeing though.

raymondfeng · 2018-08-27T16:10:18Z

See microsoft/TypeScript#26689. I created a very simple project to reproduce the problem at https://github.com/raymondfeng/learn-a/.

raymondfeng · 2018-09-28T14:54:30Z

I finally make it working with the following changes:

Upgrade to TypeScript 3.1.1
Reorganize the project layouts:
- Move test related files under src/__tests__
- Set up tsconfig with {"compilerOptions": {"rootDir": "src", "outDir": "dist"}}
- Add "types": "./dist/index.d.ts" to package.json
- Consolidate build into one distribution (dist) based on es2017
Rename tsconfig.build.json to tsconfig.json
Add copy-resources script and build:resources to copy non-TS files

Now we can run tsc -b from loopback-next.

raymondfeng · 2019-03-08T19:03:12Z

I have rebased this PR against latest master. PTAL.

bajtos · 2019-03-19T08:04:44Z

docs/site/Testing-your-application.md

@@ -129,7 +129,7 @@ helper method; for example:
 {% include code-caption.html content="src/__tests__/helpers/database.helpers.ts" %}

 ```ts
-import {ProductRepository, CategoryRepository} from '../../src/repositories';
+import {ProductRepository, CategoryRepository} from '../../repositories';


Let's land these documentation fixes in a standalone pull request please.

bajtos · 2019-03-19T08:06:10Z

packages/build/bin/copy-resources.js

+========
+
+Usage:
+  node ./bin/copy-resources


Let's land this new build helper in a new pull request. Should we include any documentation for this new helper, e.g. a new section in @loopback/build's README?

Also, shouldn't the usage line mention the actual script name used by npm, which is lb-copy-resources?

bajtos · 2019-03-19T08:08:12Z

packages/cli/generators/project/templates/tsconfig.json.ejs

@@ -16,17 +16,17 @@
    "strictNullChecks": true,
    "resolveJsonModule": true,

-    "lib": ["es2018", "dom", "esnext.asynciterable"],
+    "lib": ["es2017", "dom", "esnext.asynciterable"],


We want to keep using ES2018 features in our codebase, even if they are transpiled down to ES2017. Please revert this line.

bajtos · 2019-03-19T08:58:06Z

benchmark/package.json

@@ -8,12 +8,13 @@
    "performance",
    "benchmark"
  ],
-  "main": "index.js",
+  "types": "./dist/index.d.ts",


I'd like to make the change in types configuration in a standalone pull request because it requires changes in other places too. See #2613

bajtos · 2019-05-10T12:21:23Z

This pull request is pretty much blocked by microsoft/TypeScript#30823 :(

Refactor-rename does not work across packages.

Find all references does not work either.

Configure TSC to emit declaration maps, providing information needed by IDEs like VSCode to jump from usage of a symbol to its definition in the original source code. Simplify layout of all packages, remove top-level `index.ts` and `index.d.ts` files in favor of a package.json entry "types": "dist/index.d.ts" Simplify layout of module packages (excluding example applications) and remove the top-level `index.js` file in favor of a package.json entry "main": "dist/index.js" Update developer documentation and project templates accordingly. Benefits of these changes: - It's a preparation step for TypeScript Project References - Build is consuming less CPU cycles now. User time used to be 5m21 before, it's 1m48s now. Real time went down from 0m49s to 0m37s. - Simpler project setup in our packages, we no longer need to maintain dummy index files at the top level. Drawbacks: - Refactor-rename does not work across packagas. I believe this is a limitation of TypeScript, hopefully it will be eventually fixed. - Navigation across packages may not work until the monorepo is built.

- add script to build typescript project references - reorg project layout for better typescript refs - switch to tsconfig.json - add copy-resources script

raymondfeng requested a review from bajtos as a code owner August 22, 2018 20:12

raymondfeng changed the title ~~chore: add script to build typescript project references~~ [WIP] chore: add script to build typescript project references Aug 22, 2018

raymondfeng mentioned this pull request Aug 22, 2018

Fix lerna packages #1635

Merged

7 tasks

raymondfeng force-pushed the add-ts-project-refs branch 3 times, most recently from b060100 to 9afd744 Compare August 22, 2018 20:31

bajtos reviewed Aug 23, 2018

View reviewed changes

bin/update-project-refs.js Outdated Show resolved Hide resolved

bajtos reviewed Aug 23, 2018

View reviewed changes

bin/update-project-refs.js Outdated Show resolved Hide resolved

raymondfeng force-pushed the add-ts-project-refs branch 2 times, most recently from ee3cf82 to 1e6dd21 Compare August 23, 2018 23:08

dhmlau assigned raymondfeng Sep 8, 2018

raymondfeng force-pushed the add-ts-project-refs branch 2 times, most recently from 47d11a1 to 729be0a Compare September 28, 2018 04:54

raymondfeng requested review from b-admike, hacksparrow, jannyHou, marioestradarosa and virkt25 as code owners September 28, 2018 04:54

raymondfeng force-pushed the add-ts-project-refs branch 2 times, most recently from 3c811ae to 63a0cca Compare September 28, 2018 14:40

raymondfeng changed the title ~~[WIP] chore: add script to build typescript project references~~ chore: add script to build typescript project references Sep 28, 2018

bajtos mentioned this pull request Jan 4, 2019

refactor: rename tsconfig.build files in examples #2201

Merged

7 tasks

bajtos mentioned this pull request Jan 24, 2019

build(testlab): move test files to src/__tests__ #2283

Merged

3 tasks

bajtos added the Internal Tooling Issues related to our tooling and monorepo infrastructore label Jan 24, 2019

This was referenced Feb 1, 2019

build: move test files to src/__tests__ in example apps #2316

Merged

build: move test files to src/__tests__ in packages #2330

Merged

Scaffold test files to src/__tests__ + small fixes #2339

Merged

bajtos added the feature label Feb 12, 2019

raymondfeng force-pushed the add-ts-project-refs branch from 9e41fbf to 9866136 Compare March 8, 2019 19:02

raymondfeng requested a review from bajtos March 8, 2019 19:02

This was referenced Mar 18, 2019

Enable TypeScript Project References #2609

Closed

feat: enable declarationMap, use .d.ts files in monorepo too #2613

Closed

bajtos reviewed Mar 19, 2019

View reviewed changes

raymondfeng force-pushed the add-ts-project-refs branch from 9866136 to 86dddfe Compare March 30, 2019 04:16

raymondfeng requested a review from nabdelgadir as a code owner March 30, 2019 04:16

raymondfeng force-pushed the add-ts-project-refs branch 3 times, most recently from 06df8fc to e08f02e Compare April 2, 2019 21:31

raymondfeng force-pushed the add-ts-project-refs branch 2 times, most recently from 686efa7 to fbdf3be Compare April 17, 2019 20:13

This was referenced May 20, 2019

Thoughts on Loopback & the Ecosystem #2921

Closed

feat(build): enable incremental compilation #2928

Merged

raymondfeng force-pushed the add-ts-project-refs branch from fbdf3be to 7b50297 Compare May 23, 2019 22:04

raymondfeng force-pushed the add-ts-project-refs branch from 7b50297 to f3639df Compare June 3, 2019 16:58

feat: enable support for typescript references

9bb6296

- add script to build typescript project references - reorg project layout for better typescript refs - switch to tsconfig.json - add copy-resources script

raymondfeng force-pushed the add-ts-project-refs branch from f3639df to 9bb6296 Compare June 3, 2019 17:01

raymondfeng mentioned this pull request Apr 19, 2020

feat: enable TypeScript project references #5155

Merged

7 tasks

raymondfeng closed this Apr 19, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: add script to build typescript project references #1636

chore: add script to build typescript project references #1636

raymondfeng commented Aug 22, 2018 •

edited

Loading

bajtos Aug 23, 2018

bajtos Aug 23, 2018

raymondfeng Aug 23, 2018

bajtos Aug 23, 2018

bajtos commented Aug 23, 2018

raymondfeng commented Aug 23, 2018

bajtos commented Aug 23, 2018

raymondfeng commented Aug 23, 2018

raymondfeng commented Aug 24, 2018

bajtos commented Aug 27, 2018

raymondfeng commented Aug 27, 2018

raymondfeng commented Sep 28, 2018 •

edited

Loading

raymondfeng commented Mar 8, 2019

bajtos Mar 19, 2019

bajtos Mar 19, 2019

bajtos Mar 19, 2019

bajtos Mar 19, 2019

bajtos commented May 10, 2019

chore: add script to build typescript project references #1636

chore: add script to build typescript project references #1636

Conversation

raymondfeng commented Aug 22, 2018 • edited Loading

Checklist

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

bajtos commented Aug 23, 2018

raymondfeng commented Aug 23, 2018

bajtos commented Aug 23, 2018

raymondfeng commented Aug 23, 2018

raymondfeng commented Aug 24, 2018

bajtos commented Aug 27, 2018

raymondfeng commented Aug 27, 2018

raymondfeng commented Sep 28, 2018 • edited Loading

raymondfeng commented Mar 8, 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

bajtos commented May 10, 2019

raymondfeng commented Aug 22, 2018 •

edited

Loading

raymondfeng commented Sep 28, 2018 •

edited

Loading