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

chore!: update typescript to version 5.6.3 #5145

Open
wants to merge 16 commits into
base: next
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 0 additions & 36 deletions .github/workflows/unit-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -129,39 +129,3 @@ jobs:
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
with:
verbose: true
api-eol-node-test:
Copy link
Contributor Author

@david-luna david-luna Nov 18, 2024

Choose a reason for hiding this comment

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

note for reviewer: TypeScript 5.0+ removes support for Node.js <=12.0

strategy:
fail-fast: false
matrix:
node_version:
- "8"
- "10"
- "12"
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v4

- uses: actions/setup-node@v4
with:
cache: 'npm'
cache-dependency-path: |
package-lock.json
node-version: ${{ matrix.node_version }}

- name: Build
working-directory: ./api
run: |
npm install --ignore-scripts
npm install @types/mocha@^7 mocha@^7 ts-loader@^8
node ../scripts/version-update.js
npx tsc --build tsconfig.json tsconfig.esm.json

- name: Test
working-directory: ./api
# running test:eol as node 8 is not supported anymore by the version of nyc we use, as we don't report coverage
# for this step we leave it out.
# Details: nyc requires istanbul-lib-report, which silently dropped support for Node.js v8 when going from
# 3.0.0 to 3.0.1 by requiring make-dir@^4.0.0 to fix https://github.com/advisories/GHSA-c2qf-rxjj-qqgw.
# make-dir does not support Node.js v8 anymore.
run: npm run test:eol
2 changes: 1 addition & 1 deletion .mocharc.yml
Original file line number Diff line number Diff line change
@@ -1 +1 @@
require: 'ts-node/register'
require: 'ts-node/register/transpile-only'
1 change: 1 addition & 0 deletions CHANGELOG_NEXT.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,6 @@
### :house: (Internal)

* chore: remove checks for unsupported node versions [#4341](https://github.com/open-telemetry/opentelemetry-js/pull/4341) @dyladan
* chore: update typescript to version `^5.6.3` [#5145](https://github.com/open-telemetry/opentelemetry-js/pull/5145) @david-luna
Copy link
Contributor

Choose a reason for hiding this comment

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

This almost certainly should move up to the "breaking change" section, no?

Also should that say 5.6.3 rather than ^5.6.3? The package.json files below (at least some of them) pin the version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At 1st I checked https://github.com/microsoft/TypeScript/wiki/Breaking-Changes and was hesitating since I'm not completely sure how the produced types may break user apps. Also I've tried using ^ in dependencies, its a leftover.

Thanks for spotting it. I'll move it up :)


### :bug: (Bug Fix)
14 changes: 14 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,20 @@ cd packages/opentelemetry-module-name
npm run watch
```

#### TypeScript version

TypeScript version used to compile the pacakges is `v5.6.3`. If you plan to make your own instrumentation script
in a `.ts` file it is recommended to use same version or higher.

<!-- TODO: review the update policy -->
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note for reviewer: any other items we can add to this list?

Also TypeScript is meant to be updated in compatible verisons of `5.x`. However there could be scenarios
where we might don't want to update the minor version:
Copy link
Contributor

Choose a reason for hiding this comment

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

This is likely my TypeScript-ignorance: What is TypeScript's definition of "compatible versions", if not all of "5.x"?

Or could I be misunderstanding what you are saying? Are you saying "all updates of TS 5.x to a later 5.x version is meant to be compatible, but here is a list of reasons that might not be true for us: ..."?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sometimes a TS release has this paragraph that makes me be cautious about updating versions.
See: https://devblogs.microsoft.com/typescript/announcing-typescript-5-0/#lib.d.ts-changes


- breaking changes in `lib.d.ts`: each release of TypeScript may come with a section of changes
in `lib.d.ts` file ([example](https://devblogs.microsoft.com/typescript/announcing-typescript-5-5/#lib.d.ts-changes)). These changes
include DOM and other platform types that, if breaking, could break our compilation or even compilation
of our coonsumers if re-export them.

### Running tests

Similar to compilations, tests can be run from the root to run all tests or from a single module to run only the tests for that module.
Expand Down
5 changes: 2 additions & 3 deletions api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
"lint": "eslint . --ext .ts",
"test:browser": "karma start --single-run",
"test": "nyc mocha 'test/**/*.test.ts'",
"test:eol": "mocha 'test/**/*.test.ts'",
"test:webworker": "karma start karma.worker.js --single-run",
"cycle-check": "dpdm --exit-code circular:1 src/index.ts",
"version": "node ../scripts/version-update.js",
Expand Down Expand Up @@ -77,7 +76,7 @@
"access": "public"
},
"devDependencies": {
"@types/mocha": "10.0.8",
"@types/mocha": "10.0.9",
"@types/node": "18.6.5",
"@types/sinon": "17.0.3",
"@types/webpack": "5.28.5",
Expand All @@ -98,7 +97,7 @@
"nyc": "15.1.0",
"sinon": "15.1.2",
"ts-loader": "9.5.1",
"typescript": "4.4.4",
"typescript": "5.6.3",
Copy link
Member

Choose a reason for hiding this comment

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

@open-telemetry/javascript-maintainers what do you think about making this change in the API package? I think we should probably just go for it:

  1. We can't stay on 4.4.4 forever
  2. We don't want to go to API 2.0 any time in the foreseeable future

I think for both of these to be true, we have to eventually make this change in a minor API version, painful though it may be to some small number of users.

In order to not make this change, we'd have to keep a different version of typescript here than everywhere else. IDK how painful this would be in practice in our monorepo (maybe not that bad?)

Copy link
Member

@pichlermarc pichlermarc Nov 26, 2024

Choose a reason for hiding this comment

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

Hmm, I think at least typedoc is coupled with the typescript version - so it may hold us back with quite a few dependencies as well. I'm for updating. Maybe we could do the following:

  • we compile using [email protected]
  • we add a integration test package (not linked to the monorepo) that installs the local @opentelemetry/api compiled with 5.6.3, and that checks if current features compile with TypeScript 4.4.4 and only runs in the test workflow. Maybe it even works fine since we're not using any new TypeScript features. 🤔
    • If it does work fine the question might become: do we want to allow new API features that use new TypeScript features that would not work with 4.4.4 in minor versions?

Here's me hoping that bumping the API major will be allowed at some point in the future.

Edit: the reason we don't do is because of this spec, and 2.0 milestone scope creep, right? Dropping language version support has its own spec actually and says that we should follow the conventions given by the ecosystem (which would be bumping major). This spec does not explicitly prohibit it.

"unionfs": "4.5.4",
"webpack": "5.94.0"
},
Expand Down
2 changes: 1 addition & 1 deletion api/test/common/diag/logLevel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ describe('LogLevelFilter DiagLogger', () => {

const levelMap: Array<{
message: string;
level: DiagLogLevel;
level: number;
ignoreFuncs: Array<keyof DiagLogger>;
}> = [
{ message: 'ALL', level: DiagLogLevel.ALL, ignoreFuncs: [] },
Expand Down
2 changes: 1 addition & 1 deletion examples/esm-http-ts/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
/* Modules */
"module": "ESNext" /* Specify what module code is generated. */,
"rootDir": "." /* Specify the root folder within your source files. */,
"moduleResolution": "node" /* Specify how TypeScript looks up a file from a given module specifier. */,
"moduleResolution": "nodenext" /* Specify how TypeScript looks up a file from a given module specifier. */,
"resolveJsonModule": true /* Enable importing .json files. */,

/* Emit */
Expand Down
2 changes: 1 addition & 1 deletion examples/opentelemetry-web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
"@babel/preset-env": "^7.22.20",
"babel-loader": "^8.0.6",
"ts-loader": "^9.2.6",
"typescript": "^4.5.2",
"typescript": "5.6.3",
"webpack": "^5.89.0",
"webpack-cli": "^5.1.4",
"webpack-dev-server": "^4.5.0",
Expand Down
2 changes: 1 addition & 1 deletion experimental/backwards-compatibility/node14/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
},
"devDependencies": {
"@types/node": "14.18.25",
"typescript": "4.4.4"
"typescript": "5.6.3"
},
"author": "OpenTelemetry Authors",
"license": "Apache-2.0",
Expand Down
2 changes: 1 addition & 1 deletion experimental/backwards-compatibility/node16/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
},
"devDependencies": {
"@types/node": "16.11.52",
"typescript": "4.4.4"
"typescript": "5.6.3"
},
"author": "OpenTelemetry Authors",
"license": "Apache-2.0",
Expand Down
4 changes: 2 additions & 2 deletions experimental/packages/api-events/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
"@opentelemetry/api-logs": "0.54.2"
},
"devDependencies": {
"@types/mocha": "10.0.8",
"@types/mocha": "10.0.9",
"@types/node": "18.6.5",
"@types/webpack-env": "1.16.3",
"babel-plugin-istanbul": "7.0.0",
Expand All @@ -79,7 +79,7 @@
"mocha": "10.7.3",
"nyc": "15.1.0",
"ts-loader": "9.5.1",
"typescript": "4.4.4",
"typescript": "5.6.3",
"webpack": "5.94.0"
},
"homepage": "https://github.com/open-telemetry/opentelemetry-js/tree/main/experimental/packages/api-events",
Expand Down
4 changes: 2 additions & 2 deletions experimental/packages/api-logs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
"@opentelemetry/api": "^1.3.0"
},
"devDependencies": {
"@types/mocha": "10.0.8",
"@types/mocha": "10.0.9",
"@types/node": "18.6.5",
"@types/webpack-env": "1.16.3",
"babel-plugin-istanbul": "7.0.0",
Expand All @@ -78,7 +78,7 @@
"mocha": "10.7.3",
"nyc": "15.1.0",
"ts-loader": "9.5.1",
"typescript": "4.4.4",
"typescript": "5.6.3",
"webpack": "5.94.0"
},
"homepage": "https://github.com/open-telemetry/opentelemetry-js/tree/main/experimental/packages/api-logs",
Expand Down
4 changes: 2 additions & 2 deletions experimental/packages/exporter-logs-otlp-grpc/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
"@opentelemetry/api-logs": "0.54.2",
"@opentelemetry/otlp-exporter-base": "0.54.2",
"@opentelemetry/resources": "1.27.0",
"@types/mocha": "10.0.8",
"@types/mocha": "10.0.9",
"@types/node": "18.6.5",
"@types/sinon": "17.0.3",
"cross-var": "1.1.0",
Expand All @@ -63,7 +63,7 @@
"nyc": "15.1.0",
"sinon": "15.1.2",
"ts-loader": "9.5.1",
"typescript": "4.4.4"
"typescript": "5.6.3"
},
"peerDependencies": {
"@opentelemetry/api": "^1.3.0"
Expand Down
4 changes: 2 additions & 2 deletions experimental/packages/exporter-logs-otlp-http/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
"@babel/preset-env": "7.25.4",
"@opentelemetry/api": "1.9.0",
"@opentelemetry/resources": "1.27.0",
"@types/mocha": "10.0.8",
"@types/mocha": "10.0.9",
"@types/node": "18.6.5",
"@types/sinon": "17.0.3",
"@types/webpack-env": "1.16.3",
Expand All @@ -92,7 +92,7 @@
"nyc": "15.1.0",
"sinon": "15.1.2",
"ts-loader": "9.5.1",
"typescript": "4.4.4",
"typescript": "5.6.3",
"webpack": "5.94.0",
"webpack-cli": "5.1.4",
"webpack-merge": "5.10.0"
Expand Down
4 changes: 2 additions & 2 deletions experimental/packages/exporter-logs-otlp-proto/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
"@babel/core": "7.25.2",
"@babel/preset-env": "7.25.4",
"@opentelemetry/api": "1.9.0",
"@types/mocha": "10.0.8",
"@types/mocha": "10.0.9",
"@types/node": "18.6.5",
"@types/sinon": "17.0.3",
"babel-plugin-istanbul": "7.0.0",
Expand All @@ -82,7 +82,7 @@
"nyc": "15.1.0",
"sinon": "15.1.2",
"ts-loader": "9.5.1",
"typescript": "4.4.4",
"typescript": "5.6.3",
"webpack": "5.94.0",
"webpack-cli": "5.1.4",
"webpack-merge": "5.10.0"
Expand Down
4 changes: 2 additions & 2 deletions experimental/packages/exporter-trace-otlp-grpc/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
"@grpc/proto-loader": "^0.7.10",
"@opentelemetry/api": "1.9.0",
"@opentelemetry/otlp-exporter-base": "0.54.2",
"@types/mocha": "10.0.8",
"@types/mocha": "10.0.9",
"@types/node": "18.6.5",
"@types/sinon": "17.0.3",
"cross-var": "1.1.0",
Expand All @@ -60,7 +60,7 @@
"nyc": "15.1.0",
"sinon": "15.1.2",
"ts-loader": "9.5.1",
"typescript": "4.4.4"
"typescript": "5.6.3"
},
"peerDependencies": {
"@opentelemetry/api": "^1.3.0"
Expand Down
4 changes: 2 additions & 2 deletions experimental/packages/exporter-trace-otlp-http/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
"@babel/core": "7.25.2",
"@babel/preset-env": "7.25.4",
"@opentelemetry/api": "1.9.0",
"@types/mocha": "10.0.8",
"@types/mocha": "10.0.9",
"@types/node": "18.6.5",
"@types/sinon": "17.0.3",
"@types/webpack-env": "1.16.3",
Expand All @@ -83,7 +83,7 @@
"nyc": "15.1.0",
"sinon": "15.1.2",
"ts-loader": "9.5.1",
"typescript": "4.4.4",
"typescript": "5.6.3",
"webpack": "5.94.0",
"webpack-cli": "5.1.4",
"webpack-merge": "5.10.0"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
"@babel/core": "7.25.2",
"@babel/preset-env": "7.25.4",
"@opentelemetry/api": "1.9.0",
"@types/mocha": "10.0.8",
"@types/mocha": "10.0.9",
"@types/node": "18.6.5",
"@types/sinon": "17.0.3",
"babel-plugin-istanbul": "7.0.0",
Expand All @@ -81,7 +81,7 @@
"nyc": "15.1.0",
"sinon": "15.1.2",
"ts-loader": "9.5.1",
"typescript": "4.4.4",
"typescript": "5.6.3",
"webpack": "5.94.0",
"webpack-cli": "5.1.4",
"webpack-merge": "5.10.0"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
"@babel/core": "7.25.2",
"@babel/preset-env": "7.25.4",
"@opentelemetry/api": "1.9.0",
"@types/mocha": "10.0.8",
"@types/mocha": "10.0.9",
"@types/node": "18.6.5",
"@types/sinon": "17.0.3",
"babel-plugin-istanbul": "7.0.0",
Expand All @@ -72,7 +72,7 @@
"nyc": "15.1.0",
"sinon": "15.1.2",
"ts-loader": "9.5.1",
"typescript": "4.4.4",
"typescript": "5.6.3",
"webpack": "5.94.0",
"webpack-cli": "5.1.4",
"webpack-merge": "5.10.0"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
"devDependencies": {
"@grpc/proto-loader": "^0.7.10",
"@opentelemetry/api": "1.9.0",
"@types/mocha": "10.0.8",
"@types/mocha": "10.0.9",
"@types/node": "18.6.5",
"@types/sinon": "17.0.3",
"cross-var": "1.1.0",
Expand All @@ -59,7 +59,7 @@
"nyc": "15.1.0",
"sinon": "15.1.2",
"ts-loader": "9.5.1",
"typescript": "4.4.4"
"typescript": "5.6.3"
},
"peerDependencies": {
"@opentelemetry/api": "^1.3.0"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
"@babel/core": "7.25.2",
"@babel/preset-env": "7.25.4",
"@opentelemetry/api": "1.9.0",
"@types/mocha": "10.0.8",
"@types/mocha": "10.0.9",
"@types/node": "18.6.5",
"@types/sinon": "17.0.3",
"@types/webpack-env": "1.16.3",
Expand All @@ -83,7 +83,7 @@
"nyc": "15.1.0",
"sinon": "15.1.2",
"ts-loader": "9.5.1",
"typescript": "4.4.4",
"typescript": "5.6.3",
"webpack": "5.94.0",
"webpack-cli": "5.1.4",
"webpack-merge": "5.10.0"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
},
"devDependencies": {
"@opentelemetry/api": "1.9.0",
"@types/mocha": "10.0.8",
"@types/mocha": "10.0.9",
"@types/node": "18.6.5",
"@types/sinon": "17.0.3",
"cross-var": "1.1.0",
Expand All @@ -65,7 +65,7 @@
"nyc": "15.1.0",
"sinon": "15.1.2",
"ts-loader": "9.5.1",
"typescript": "4.4.4"
"typescript": "5.6.3"
},
"peerDependencies": {
"@opentelemetry/api": "^1.3.0"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,15 @@
"devDependencies": {
"@opentelemetry/api": "1.9.0",
"@opentelemetry/semantic-conventions": "1.27.0",
"@types/mocha": "10.0.8",
"@types/mocha": "10.0.9",
"@types/node": "18.6.5",
"@types/sinon": "17.0.3",
"cross-var": "1.1.0",
"lerna": "6.6.2",
"mocha": "10.7.3",
"nyc": "15.1.0",
"sinon": "15.1.2",
"typescript": "4.4.4"
"typescript": "5.6.3"
},
"peerDependencies": {
"@opentelemetry/api": "^1.3.0"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
"@opentelemetry/context-zone": "1.27.0",
"@opentelemetry/propagator-b3": "1.27.0",
"@opentelemetry/sdk-trace-base": "1.27.0",
"@types/mocha": "10.0.8",
"@types/mocha": "10.0.9",
"@types/node": "18.6.5",
"@types/sinon": "17.0.3",
"@types/webpack-env": "1.16.3",
Expand All @@ -78,7 +78,7 @@
"nyc": "15.1.0",
"sinon": "15.1.2",
"ts-loader": "9.5.1",
"typescript": "4.4.4",
"typescript": "5.6.3",
"webpack": "5.94.0",
"webpack-cli": "5.1.4",
"webpack-merge": "5.10.0"
Expand Down
Loading
Loading