Repository navigation
Conversation
Preserve the Java-facing extraction protocol while adopting the pinned native TypeScript API and platform-specific runtimes. Add compatibility, lifecycle, packaging, and syntax regression coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 86598cdc-5945-4d8e-9ea4-eeb4eb4356ba
![github-license-compliance[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Pass wrapper and native archive entries to Bazel zipper through response files instead of expanding every path on the command line. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 86598cdc-5945-4d8e-9ea4-eeb4eb4356ba
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The production parser and packaging overhaul still lacks Linux and Windows execution coverage and needs human review.
3 open issues
2 files not reviewed
javascript/extractor/lib/typescript/package-lock.json: Generated filejavascript/extractor/lib/typescript/ts7/package-lock.json: Generated file
What changed in this PR
Migrates the JavaScript extractor’s TypeScript parser to native 7.1 while preserving its Java-facing AST protocol.
Changes:
- Ports AST projection and manages native resources per file.
- Separates common JavaScript files from platform-specific executables.
- Adds compatibility, lifecycle, packaging, and diagnostic coverage.
| File | Description |
|---|---|
misc/bazel/pkg.bzl |
Supports architecture-specific imported ZIPs. |
javascript/ql/test/library-tests/TypeScript/SourceImports/tst.ts |
Adds source-phase import fixture. |
javascript/ql/test/library-tests/TypeScript/SourceImports/test.ql |
Checks imports and references. |
javascript/ql/test/library-tests/TypeScript/SourceImports/test.expected |
Records expected bindings. |
javascript/ql/integration-tests/diagnostics/internal-error/test.py |
Enables injected native failure. |
javascript/ql/integration-tests/diagnostics/internal-error/fail-native.cjs |
Terminates the spawned child. |
javascript/extractor/tests/ts/output/trap/import-attributes.ts.trap |
Records import-attribute extraction. |
javascript/extractor/test/com/semmle/js/extractor/test/BUILD.bazel |
Adds native runtime test inputs. |
javascript/extractor/test/com/semmle/js/extractor/test/AllTests.java |
Installs ZIPs with executable permissions. |
javascript/extractor/lib/typescript/tsconfig.json |
Updates compilation for ESM. |
javascript/extractor/lib/typescript/ts7/wrapper.test.mjs |
Tests protocol compatibility. |
javascript/extractor/lib/typescript/ts7/probe.mjs |
Probes native API compatibility. |
javascript/extractor/lib/typescript/ts7/package.json |
Pins probe dependencies. |
javascript/extractor/lib/typescript/ts7/package-lock.json |
Locks probe packages. |
javascript/extractor/lib/typescript/test/package_test.py |
Tests installed runtime archives. |
javascript/extractor/lib/typescript/test/lifecycle.test.mjs |
Tests process lifecycle and failures. |
javascript/extractor/lib/typescript/src/virtual_source_root.ts |
Removes legacy source-root mapping. |
javascript/extractor/lib/typescript/src/typescript.js |
Loads native API entry points. |
javascript/extractor/lib/typescript/src/typescript.d.ts |
Declares native API exports. |
javascript/extractor/lib/typescript/src/main.ts |
Implements per-file native parsing. |
javascript/extractor/lib/typescript/src/common.ts |
Removes legacy project management. |
javascript/extractor/lib/typescript/src/ast_extractor.ts |
Projects native ASTs into JSON. |
javascript/extractor/lib/typescript/README.md |
Documents migration and performance tradeoffs. |
javascript/extractor/lib/typescript/package.json |
Updates runtime and build commands. |
javascript/extractor/lib/typescript/package-runtime.mjs |
Selects and stages native executables. |
javascript/extractor/lib/typescript/package-lock.json |
Locks production runtime packages. |
javascript/extractor/lib/typescript/BUILD.bazel |
Builds split archives and packaging tests. |
javascript/BUILD.bazel |
Includes native packages in distributions. |
Review effort: Max
Comment on lines
+5
to
+6
| "engines": { | ||
| "node": ">=18.15.0" |
| assert.match(version, /^7\.1\.0-dev\./); | ||
| assert.equal(require("typescript").version, version); | ||
| assert.equal(require("typescript-legacy").version, "5.9.2"); | ||
| const cli = join(dirname(require.resolve("typescript/package.json")), "bin/tsc"); |
|
|
||
|
|
||
| def test(codeql, javascript, cwd): | ||
| os.environ["NODE_OPTIONS"] = f'--require "{cwd / "fail-native.cjs"}"' |
Use the existing wrapper exit-code classification after IPC errors, avoiding a race between native prefetch failure and Java's next request. Preserve the communication-error fallback for a live wrapper. Record successful Linux and Windows runtime execution coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 86598cdc-5945-4d8e-9ea4-eeb4eb4356ba
paldepind
marked this pull request as draft
October 19, 2026 11:09
asgerf
reviewed
Oct 19, 2026
| $end: node.end, | ||
| }; | ||
| const properties = node as ts.Node & { [property: string]: unknown }; | ||
| for (const property of astProperties) { |
Contributor
There was a problem hiding this comment.
Looping over the whole astProperties array for every node seems very slow. Could we replace this with a guarded for..in loop or equivalent?
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrate the JavaScript/TypeScript extractor from TypeScript 5.9.2 to the pinned native
typescript@7.1.0-dev.20261015.1npm package.typescript/syncandtypescript/ast, explicitly projecting native AST getters into the existing Java-facing JSON protocol.Validation
Only the reviewed import-attribute TRAP expectation changed; no existing QL expectations changed.
Performance tradeoff
On macOS arm64 with Node.js 18.15.0, median parser/protocol time over three runs increased from 0.237 s to 1.349 s for 103 tiny fixture files, and from 0.991 s to 1.116 s for five parses of a 588 KB declaration file. Sampled combined Node-plus-native RSS was slightly lower. The per-file native-process lifetime is retained deliberately to bound resources; this migration does not claim a speedup. Details and measurement scope are documented in the wrapper README.