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

Isolated declarations parallel builder #95

Merged

Conversation

dragomirtitian
Copy link

@dragomirtitian dragomirtitian commented Sep 25, 2023

No description provided.

…tterns.

Signed-off-by: Titian Cernicova-Dragomir <tcernicovad1@bloomberg.net>
Signed-off-by: Titian Cernicova-Dragomir <tcernicovad1@bloomberg.net>
Signed-off-by: Titian Cernicova-Dragomir <tcernicovad1@bloomberg.net>
@dragomirtitian dragomirtitian merged commit 35d400e into isolated-declarations Sep 25, 2023
1 check passed
@@ -0,0 +1,57 @@
1. Build typescript
```sh
cd typescript
Copy link

@lazytype lazytype Sep 30, 2023

Choose a reason for hiding this comment

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

nit: The repo directory is more likely to be named TypeScript rather than typescript if one is doing a default git clone operation.

It might be worth mentioning that every directory with a package.json needs to have an npm install first.

Also, I couldn't actually get this to work in my composite TS project. For each file an error like the following is emitted:

Output file '<path_to_d_ts_file>.d.ts' has not been built from source file '<path_to_tsx_file>.tsx.

Nice to have: A flag to increase node max-old-space-size for each worker process.

Copy link
Author

Choose a reason for hiding this comment

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

@lazytype On which run did you get the error Output file '<path_to_d_ts_file>.d.ts' has not been built from source file '<path_to_tsx_file>.tsx.? The task file should contain each project with correct dependencies (which for a topological build should be the referenced composite projects, and for the isolated declarations versions it should be the declarations of the referenced projects). There might be a bug in the task file generator.

Choose a reason for hiding this comment

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

Sorry for the late reply, I have been busy. I tried to build again using the tip of isolated-declarations, but the build appears to be in a non-functional state.

  • The scripts in parallel-build/package.json fail, does it need something like this?
diff --git a/parallel-build/package.json b/parallel-build/package.json
index 029e18a56..8cba9dd59 100644
--- a/parallel-build/package.json
+++ b/parallel-build/package.json
@@ -5,8 +5,8 @@
     "main": "index.js",
     "type": "module",
     "scripts": {
-        "build": "tsc -p ./",
-        "watch": "tsc -w -p ./"
+        "build": "node ../built/local/tsc.js -p ./",
+        "watch": "node ../built/local/tsc.js -w -p ./"
     },
     "author": "",
     "license": "ISC",
  • Running npm run build in parallel-build/ fails because it can't find modules that have moved. The functions they're trying to import also don't exist anymore or were moved/renamed.
> parallel-build@1.0.0 build
> node ../built/local/tsc.js -p ./

src/worker-utils/build-declarations.ts:5:8 - error TS2307: Cannot find module 'external-declarations/build/compiler/transform-project.js' or its corresponding type declarations.

5 } from "external-declarations/build/compiler/transform-project.js";
         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

src/worker-utils/build-tsc-shard.ts:7:8 - error TS2307: Cannot find module 'external-declarations/build/compiler/transform-project.js' or its corresponding type declarations.

7 } from "external-declarations/build/compiler/transform-project.js";
         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Found 2 errors in 2 files.

Errors  Files
     1  src/worker-utils/build-declarations.ts:5
     1  src/worker-utils/build-tsc-shard.ts:7

Comment on lines +98 to +101
function cleanAST(node: Node) {
(node as any).id = undefined;
ts.forEachChild(node, cleanAST, nodes => nodes.forEach(cleanAST));
}
Copy link

Choose a reason for hiding this comment

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

This looks like an interesting trick. Does setting node.id to undefined trigger some kind of cache invalidation, e.g. the import resolution cache?

@dragomirtitian dragomirtitian deleted the isolated-declarations-parallel-builder branch November 17, 2023 15:56
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.

3 participants