Closed Clement-Stauner closed 6 months ago
@Clement-Stauner thanks for reaching out. I've tried reproducing this (https://github.com/pichlermarc/repro-4489) but I'm not sure I'm missing something. Would you mind trying to remove node_modules
and package-lock.json
and trying again with a clean install of dependencies?
I managed to solve the issue by setting the TS compiler skipLibCheck
option to true
.
{
"compilerOptions": {
"skipLibCheck": true
}
}
But there are still two native type safety bugs in the library that the TS compiler caught while trying to compile, it you set strong type checking in the tsconfig.json
file, you will get the error.
Here here my tsconfig.json
file that caught the two bugs with skipLibCheck
option to false
.
{
"compilerOptions": {
"baseUrl": "./", // Add this line
"paths": {
// Add or adjust this section
"*": ["node_modules/*", "src/*"]
},
"module": "CommonJS", // Ensure this is set for Node.js compatibility
"rootDirs": ["src", "dist"], // Add this line if you want to include multiple root directories
"outDir": "dist",
"noEmitOnError": true,
"forceConsistentCasingInFileNames": true,
"esModuleInterop": true,
"isolatedModules": true,
"moduleResolution": "node",
/* Type Checking */
"strict": true /* Enable all strict type-checking options. */,
"noImplicitAny": true /* Enable error reporting for expressions and declarations with an implied 'any' type. */,
"strictNullChecks": true /* When type checking, take into account 'null' and 'undefined'. */,
"strictFunctionTypes": true /* When assigning functions, check to ensure parameters and the return values are subtype-compatible. */,
"strictBindCallApply": true /* Check that the arguments for 'bind', 'call', and 'apply' methods match the original function. */,
"strictPropertyInitialization": true /* Check for class properties that are declared but not set in the constructor. */,
"noImplicitThis": true /* Enable error reporting when 'this' is given the type 'any'. */,
"useUnknownInCatchVariables": true /* Default catch clause variables as 'unknown' instead of 'any'. */,
"alwaysStrict": true /* Ensure 'use strict' is always emitted. */,
"noUnusedLocals": true /* Enable error reporting when local variables aren't read. */,
"noUnusedParameters": true /* Raise an error when a function parameter isn't read. */,
"exactOptionalPropertyTypes": true /* Interpret optional property types as written, rather than adding 'undefined'. */,
"noImplicitReturns": true /* Enable error reporting for codepaths that do not explicitly return in a function. */,
"noFallthroughCasesInSwitch": true /* Enable error reporting for fallthrough cases in switch statements. */,
"noUncheckedIndexedAccess": true /* Add 'undefined' to a type when accessed using an index. */,
"noImplicitOverride": true /* Ensure overriding members in derived classes are marked with an override modifier. */,
"noPropertyAccessFromIndexSignature": true /* Enforces using indexed accessors for keys declared using an indexed type. */,
"allowUnusedLabels": true /* Disable error reporting for unused labels. */,
"allowUnreachableCode": true /* Disable error reporting for unreachable code. */,
/* Completeness */
"skipDefaultLibCheck": true /* Skip type checking .d.ts files that are included with TypeScript. */,
"skipLibCheck": true
},
"include": ["src/**/*"]
}
Ah I see. This is actually reproducible with "exactOptionalPropertyTypes": true
, which is not included in strict
by default (because turning it on does cause a lot of churn for existing code bases to adhere to it).
My guess is that adding that will actually cause a huge diff though because we're not using optional properties like that. Not sure what the implications for this would be (if that's a breaking change for some consumers of the library, my guess is "no" but I would have to try that out). It might be a good idea to do this in SDK 2.0 at least to maximize compatibility.
I'll bring it up in the SIG meeting this week, looks like this issue is a duplicate of #3713
Closing as duplicate of #3713.
We've discussed it in the SIG meeting last week, and we're generally in favor of changing to "exactOptionalPropertyTypes": true
. PRs for this are welcome, for further prograss-tracking, please use #3713 :slightly_smiling_face:
What happened?
Steps to Reproduce
Expected Result
Builds all dependencies
Actual Result
Fail to build module
@opentelemetry/sdk-logs
Additional Details
OpenTelemetry Setup Code
package.json
Relevant log output