Bootstrap TypeScript compilation - #2849
Merged
Merged
Conversation
Contributor
Hello francoisferrand,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Codecov Report❌ Patch coverage is
Additional details and impacted files
... and 1 file with indirect coverage changes
@@ Coverage Diff @@
## development/9.6 #2849 +/- ##
===================================================
- Coverage 76.79% 76.09% -0.70%
===================================================
Files 205 206 +1
Lines 14288 14426 +138
===================================================
+ Hits 10972 10977 +5
- Misses 3306 3439 +133
Partials 10 10
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
francoisferrand
force-pushed
the
improvement/BB-295-bootstrap-typescript
branch
from
September 14, 2026 10:11
37f6969 to
53d5568
Compare
Contributor
francoisferrand
force-pushed
the
improvement/BB-295-bootstrap-typescript
branch
from
September 14, 2026 22:27
2d5d4fe to
076e82e
Compare
francoisferrand
force-pushed
the
improvement/BB-295-bootstrap-typescript
branch
2 times, most recently
from
September 15, 2026 05:17
e808f34 to
b894c5b
Compare
francoisferrand
requested review from
a team,
DarkIsDude and
SylvainSenechal
September 15, 2026 05:17
DarkIsDude
approved these changes
Sep 15, 2026
SylvainSenechal
approved these changes
Sep 15, 2026
SylvainSenechal
left a comment
SylvainSenechal
left a comment
Contributor
There was a problem hiding this comment.
I don't have much to say, if it works it works and its cool 🤷
Typescript 7 is available since July, but when I tried using it in Zenko last month there was an issue with eslint still not ready to accept it, so I guess it can wait.
Maybe there are some more config that we will want to add or change but no need to block on this pr.
We may want to do a release now and use it in Zenko to make sure nothing is broken when building the image ?
francoisferrand
force-pushed
the
improvement/BB-295-bootstrap-typescript
branch
from
September 15, 2026 15:29
b894c5b to
1959d8e
Compare
MetricsConsumer built its RedisClient as a local and handed it straight to the StatsModel, so nothing could reach it afterwards: close() only tore down the kafka consumer, and the socket stayed open for the life of the process. Keep the client as a member and disconnect it alongside the consumer. Both populators already close their metrics consumer, so there is nothing to wire up on their side. close() also dereferenced the consumer unconditionally, while start() only assigns it on the ready event: closing during startup threw a TypeError and left the caller's series waiting on a callback that never came. Guard it, and still call back. Issue: BB-882
New modules should be written in TypeScript, but the toolchain has no notion of it today. Set up the compiler and the linter so `.ts` files are understood, without changing how any existing JavaScript is built or run: this commit only adds checking, nothing is emitted yet. `allowJs` with `checkJs` off keeps the existing sources in the program for module resolution while leaving them unchecked, so the strict settings apply to new TypeScript only. `yarn typecheck` runs in the existing lint job rather than a job of its own. Issue: BB-295
TypeScript modules have to be compiled before they can run, so the image now ships the build output instead of the sources. The contents of dist/ are flattened into the working directory, which keeps lib/, bin/ and extensions/ exactly where they are today: the package scripts and every entrypoint stay untouched, and a dist/ prefix would break all of them. Compilation happens in a stage of its own so the devDependencies it needs never reach the runtime image. conf/config.json is read with readFileSync rather than required, so the compiler never emits it and it has to be copied over on its own. Checked that this leaves existing code alone: for all 201 compiled files the output parses to the same syntax tree as its source, the differences being whitespace and the source map comment. alwaysStrict is off because strict implies it, and adding the directive to the 148 modules that run without it today would change their semantics in the image only — the test suite runs against the sources and would not notice. Issue: BB-295
A module written in TypeScript is invisible to the test suites, which require their subjects directly from the sources. Registering the loader in .mocharc.json applies to every suite at once, so the thirteen mocha invocations stay as they are, and a spec keeps requiring its subject without an extension whichever language it is written in. Coverage is collected from the same sources, so lib TypeScript joins the files nyc reports on. Issue: BB-295
First module to use the new toolchain, chosen for being a leaf: two pure functions, no imports, and callers that stay in JavaScript. readUInt64BE in particular takes a Buffer and returns a Number, which the signature now states rather than leaving to a comment. It uses `export =` so that requiring it is unchanged: `export default` would force every caller to reach through `.default`. Its spec is untouched, and neither are the two callers, which is the point — a TypeScript module has to be indistinguishable from the JavaScript one it replaces. Issue: BB-295
francoisferrand
force-pushed
the
improvement/BB-295-bootstrap-typescript
branch
from
September 15, 2026 17:20
1959d8e to
09d85b1
Compare
Contributor
Author
|
/approve |
Contributor
|
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
This pull request did not target the following hotfix branch(es) so they
Please check the status of the associated issue BB-295. Goodbye francoisferrand. The following options are set: approve |
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
4 participants
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.
New modules should be writable in TypeScript, but nothing in the toolchain understands it today. This wires up the compiler, the linter, the image build and the test suites, then converts one module to prove the whole path works.
The guiding constraint was that existing code must not notice. 379 JavaScript files and 155 test files are untouched: no test renamed, no spec migrated, no script repointed.
What each commit does
Type checking —
tsconfig.jsonplus atypecheckscript, run in the existing lint job rather than a job of its own.allowJswithcheckJsoff keeps the current sources in the program so module resolution works, while leaving them unchecked, so the strict settings only ever apply to new TypeScript. typescript-eslint is scoped to**/*.ts.Compiling into the image — the image now ships the build output. The contents of
dist/are flattened into the working directory, solib/,bin/andextensions/keep the paths they have today and every package script and entrypoint stays as it is. Compilation happens in its own stage so the devDependencies it needs never reach the runtime image.conf/is copied separately because the compiler only emits JSON it seesrequired, andconf/config.jsonis read withreadFileSync. The IAM policies are not copied at all: they ship as their ownbackbeat-policiesartifact, built from the checkout.Loading TypeScript in tests — registering ts-node in
.mocharc.jsoncovers every suite at once, so the thirteen mocha invocations stay as they are and a spec keeps requiring its subject without an extension, whichever language it is written in.First module —
lib/util/buffer.jsbecomeslib/util/buffer.ts. It usesexport =, notexport default, so requiring it is unchanged;export defaultwould force every caller to reach through.default.Worth knowing for the next migration
export =is the rule, not a preference. It emits a plainmodule.exports = {...}, which is what every existing caller expects.strictimpliesalwaysStrict, so the compiler would otherwise add"use strict"to all 201 emitted files, 148 of which run sloppy today. Since mocha runs the sources and never the build output, no test would have caught the difference. It is explicitly off.When migrating, the deletion of the
.jshas to land in the same commit as the.ts. With both present the compiler emits the.tsintodist/while node resolves the.js— silently, so the image would run different code from the tests.declarationis off: backbeat ships as a service image, not a consumed library, so nothing reads its declarations. Turning it on forces type annotations into JavaScript sources to satisfy a.d.tsnobody loads.Checks
The build is a no-op for existing code: all 201 emitted files parse to the same syntax tree as their source, differing only in whitespace and the sourcemap comment.
The unit suite gives 1950 passing / 1 pending / 13 failing, identical with and without the loader registered, and identical again after the migration. The 13 need Kafka, Mongo and Redis.
tests/unit/lib/util/buffer.spec.jsis unchanged and loads the TypeScript module, as do both plain-JavaScript callers. nyc attributes coverage tobuffer.tsby name with line numbers from the source map.The image was built and compared against one from the previous Dockerfile: same behaviour, and services still read
/usr/src/app/conf/.Test renames, the jest migration and re-enabling skipped tests are deliberately out of scope; they are tracked separately.
Issue: BB-295