Tracking: fix eslint warnings #5365
Comments
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 24, 2025
/home/runner/work/opentelemetry-js/opentelemetry-js/api/src/diag/ComponentLogger.ts
36:25 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any
40:25 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any
44:24 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any
48:24 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any
52:27 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any
60:9 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any
/home/runner/work/opentelemetry-js/opentelemetry-js/api/src/experimental/trace/SugaredTracer.ts
80:5 warning 'context' is already declared in the upper scope on line 17 column 10 @typescript-eslint/no-shadow
129:5 warning 'context' is already declared in the upper scope on line 17 column 10 @typescript-eslint/no-shadow
135:5 warning 'context' is already declared in the upper scope on line 17 column 10 @typescript-eslint/no-shadow
/home/runner/work/opentelemetry-js/opentelemetry-js/api/src/trace/NoopTracer.ts
102:37 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any
Ref open-telemetry#5365
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 24, 2025
The previous interface placed the generic paramter on the interface itself. This implies that the implementors of `TextMapPropagator` can specify what carrier types it accepts. As far as I can tell, this has never been the case. The propagator API is designed to be called from the transport layers. The transport layer (caller of `inject`/`extract`) is the one who has control over what the carrier type is. The constraint is that the carrier must semantically behave as an abstract map data structure that supports setting and getting string keys/values, and the caller must supply a matching getter/setter that works with the given carrier type. For example, the fetch instrumentation calls the propogation API with one of many carrier types – POJOs, `Headers`, `Map`, etc. Therefore, a _correct_ implementation of `TextMapPropagator` must treat the carrier as opaque and only work with it using the supplied getter/setter. Unfortunately, the previous interface definition allowed a lot of sloppiness in the implementations that would cause problems in the real world. Fortunately, it seems like these all happen in tests, and all the production propagators are already compliant with the spirit of the API. This commit moves the generic parameter from the interface into the `inject` and `extract` methods. This forces implementors to comply with the API more rigiously. This is a breaking change even for compliant implementations, as it requires adjusting the method signatures to match. If an implementation supplied a custom type for the generic parameter previously existed on the interface, then that probably represents a case where it wouldn't have worked in the real world and should be refactored/rewritten to work with the abstracted carrier type as intended. Ref open-telemetry#5365
8 tasks
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 24, 2025
The previous interface placed the generic paramter on the interface itself. This implies that the implementors of `TextMapPropagator` can specify what carrier types it accepts. As far as I can tell, this has never been the case. The propagator API is designed to be called from the transport layers. The transport layer (caller of `inject`/`extract`) is the one who has control over what the carrier type is. The constraint is that the carrier must semantically behave as an abstract map data structure that supports setting and getting string keys/values, and the caller must supply a matching getter/setter that works with the given carrier type. For example, the fetch instrumentation calls the propogation API with one of many carrier types – POJOs, `Headers`, `Map`, etc. Therefore, a _correct_ implementation of `TextMapPropagator` must treat the carrier as opaque and only work with it using the supplied getter/setter. Unfortunately, the previous interface definition allowed a lot of sloppiness in the implementations that would cause problems in the real world. Fortunately, it seems like these all happen in tests, and all the production propagators are already compliant with the spirit of the API. This commit moves the generic parameter from the interface into the `inject` and `extract` methods. This forces implementors to comply with the API more rigiously. This is a breaking change even for compliant implementations, as it requires adjusting the method signatures to match. If an implementation supplied a custom type for the generic parameter previously existed on the interface, then that probably represents a case where it wouldn't have worked in the real world and should be refactored/rewritten to work with the abstracted carrier type as intended. Ref open-telemetry#5365
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 24, 2025
The current interface places the generic paramter on the interface
itself. This implies that the implementors of `TextMapPropagator`
can specify what carrier types it accepts (and that each implementor
only work with one specific type of carrier).
```ts
interface TextMapPropagator<Carrier> {
inject(context: Context, carrier: Carrier, setter: TextMapSetter<Carrier>): void;
extract(context: Context, carrier: Carrier, getter: TextMapGetter<Carrier>): void;
}
```
In reality, this is not the case.
The propagator API is designed to be called by participating code
around the various transport layers (such as the `fetch` inst on
the browser, or integration with the HTTP server library on the
backend), and it is these callers that ultimately controls what
carrier type the currently configured propagator is called with.
Therefore, a correct implementation of this interface must treat
the carrier as an opaque value, and only work with it using the
provided getter/setter.
Ideally, the interface should look like this instead:
```ts
interface TextMapPropagator {
inject<Carrier>(context: Context, carrier: Carrier, setter: TextMapSetter<Carrier>): void;
extract<Carrier>(context: Context, carrier: Carrier, getter: TextMapGetter<Carrier>): void;
}
```
This communicates and enforces the contract. Unfortunately, that
would be a breking change we are not currently prepared to make.
Instead, this commit updates the documentation to explicitly
document the discrapancy and advice implemntors the correct way
forward.
It also updates our own implementations to follow the recommended
pattern, as well as updating the tests to be more well-behaved
around this, as some of them are written to rely on this exact
behavior that would be problematic in the real world.
Ref open-telemetry#5365
Ref open-telemetry#5368
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 24, 2025
The current interface places the generic paramter on the interface
itself. This implies that the implementors of `TextMapPropagator`
can specify what carrier types it accepts (and that each implementor
only work with one specific type of carrier).
```ts
interface TextMapPropagator<Carrier> {
inject(context: Context, carrier: Carrier, setter: TextMapSetter<Carrier>): void;
extract(context: Context, carrier: Carrier, getter: TextMapGetter<Carrier>): void;
}
```
In reality, this is not the case.
The propagator API is designed to be called by participating code
around the various transport layers (such as the `fetch` inst on
the browser, or integration with the HTTP server library on the
backend), and it is these callers that ultimately controls what
carrier type the currently configured propagator is called with.
Therefore, a correct implementation of this interface must treat
the carrier as an opaque value, and only work with it using the
provided getter/setter.
Ideally, the interface should look like this instead:
```ts
interface TextMapPropagator {
inject<Carrier>(context: Context, carrier: Carrier, setter: TextMapSetter<Carrier>): void;
extract<Carrier>(context: Context, carrier: Carrier, getter: TextMapGetter<Carrier>): void;
}
```
This communicates and enforces the contract. Unfortunately, that
would be a breking change we are not currently prepared to make.
Instead, this commit updates the documentation to explicitly
document the discrapancy and advice implemntors the correct way
forward.
It also updates our own implementations to follow the recommended
pattern, as well as updating the tests to be more well-behaved
around this, as some of them are written to rely on this exact
behavior that would be problematic in the real world.
Ref open-telemetry#5365
Ref open-telemetry#5368
8 tasks
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 24, 2025
The current interface places the generic paramter on the interface
itself. This implies that the implementors of `TextMapPropagator`
can specify what carrier types it accepts (and that each implementor
only work with one specific type of carrier).
```ts
interface TextMapPropagator<Carrier> {
inject(context: Context, carrier: Carrier, setter: TextMapSetter<Carrier>): void;
extract(context: Context, carrier: Carrier, getter: TextMapGetter<Carrier>): void;
}
```
In reality, this is not the case.
The propagator API is designed to be called by participating code
around the various transport layers (such as the `fetch` inst on
the browser, or integration with the HTTP server library on the
backend), and it is these callers that ultimately controls what
carrier type the currently configured propagator is called with.
Therefore, a correct implementation of this interface must treat
the carrier as an opaque value, and only work with it using the
provided getter/setter.
Ideally, the interface should look like this instead:
```ts
interface TextMapPropagator {
inject<Carrier>(context: Context, carrier: Carrier, setter: TextMapSetter<Carrier>): void;
extract<Carrier>(context: Context, carrier: Carrier, getter: TextMapGetter<Carrier>): void;
}
```
This communicates and enforces the contract. Unfortunately, that
would be a breking change we are not currently prepared to make.
Instead, this commit updates the documentation to explicitly
document the discrapancy and advice implemntors the correct way
forward.
It also updates our own implementations to follow the recommended
pattern, as well as updating the tests to be more well-behaved
around this, as some of them are written to rely on this exact
behavior that would be problematic in the real world.
Ref open-telemetry#5365
Ref open-telemetry#5368
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 24, 2025
The current interface places the generic paramter on the interface
itself. This implies that the implementors of `TextMapPropagator`
can specify what carrier types it accepts (and that each implementor
only work with one specific type of carrier).
```ts
interface TextMapPropagator<Carrier> {
inject(context: Context, carrier: Carrier, setter: TextMapSetter<Carrier>): void;
extract(context: Context, carrier: Carrier, getter: TextMapGetter<Carrier>): void;
}
```
In reality, this is not the case.
The propagator API is designed to be called by participating code
around the various transport layers (such as the `fetch` inst on
the browser, or integration with the HTTP server library on the
backend), and it is these callers that ultimately controls what
carrier type the currently configured propagator is called with.
Therefore, a correct implementation of this interface must treat
the carrier as an opaque value, and only work with it using the
provided getter/setter.
Ideally, the interface should look like this instead:
```ts
interface TextMapPropagator {
inject<Carrier>(context: Context, carrier: Carrier, setter: TextMapSetter<Carrier>): void;
extract<Carrier>(context: Context, carrier: Carrier, getter: TextMapGetter<Carrier>): void;
}
```
This communicates and enforces the contract. Unfortunately, that
would be a breking change we are not currently prepared to make.
Instead, this commit updates the documentation to explicitly
document the discrapancy and advice implemntors the correct way
forward.
It also updates our own implementations to follow the recommended
pattern, as well as updating the tests to be more well-behaved
around this, as some of them are written to rely on this exact
behavior that would be problematic in the real world.
Ref open-telemetry#5365
Ref open-telemetry#5368
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 24, 2025
The previous interface placed the generic paramter on the interface itself. This implies that the implementors of `TextMapPropagator` can specify what carrier types it accepts. As far as I can tell, this has never been the case. The propagator API is designed to be called from the transport layers. The transport layer (caller of `inject`/`extract`) is the one who has control over what the carrier type is. The constraint is that the carrier must semantically behave as an abstract map data structure that supports setting and getting string keys/values, and the caller must supply a matching getter/setter that works with the given carrier type. For example, the fetch instrumentation calls the propogation API with one of many carrier types – POJOs, `Headers`, `Map`, etc. Therefore, a _correct_ implementation of `TextMapPropagator` must treat the carrier as opaque and only work with it using the supplied getter/setter. Unfortunately, the previous interface definition allowed a lot of sloppiness in the implementations that would cause problems in the real world. Fortunately, it seems like these all happen in tests, and all the production propagators are already compliant with the spirit of the API. This commit moves the generic parameter from the interface into the `inject` and `extract` methods. This forces implementors to comply with the API more rigiously. This is a breaking change even for compliant implementations, as it requires adjusting the method signatures to match. If an implementation supplied a custom type for the generic parameter previously existed on the interface, then that probably represents a case where it wouldn't have worked in the real world and should be refactored/rewritten to work with the abstracted carrier type as intended. Ref open-telemetry#5365
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 24, 2025
/home/runner/work/opentelemetry-js/opentelemetry-js/packages/opentelemetry-context-zone-peer-dep/src/util.ts
22:39 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any
Ref open-telemetry#5365
8 tasks
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 27, 2025
``` /home/runner/work/opentelemetry-js/opentelemetry-js/packages/opentelemetry-context-async-hooks/src/AbstractAsyncHooksContextManager.ts 72:35 warning Don't use `Function` as a type @typescript-eslint/ban-types 134:60 warning Don't use `Function` as a type @typescript-eslint/ban-types 152:64 warning Don't use `Function` as a type @typescript-eslint/ban-types 176:15 warning Don't use `Function` as a type @typescript-eslint/ban-types /home/runner/work/opentelemetry-js/opentelemetry-js/packages/opentelemetry-context-async-hooks/src/AsyncHooksContextManager.ts 49:22 warning Forbidden non-null assertion @typescript-eslint/no-non-null-assertion ``` The first set of warnings are more involved, the second one just didn't turn out to be necessary at all and the non-null assertion (the `!` postfix operator) can simply be removed. Explaination: When using `typeof foo === 'function'` TypeScript narrows the type of `foo` to `Function`, so it may be tempting to use `Function` in signatures when you want to accept any callable function. However, this is not quite what `Function` means. `Function` as a type in TypeScript has inherited a fair bit of historical baggage and behaves strangely. For the most part, it only guarentee that it has the `Function` prototype, so has things like `.name`, `.bind` and `.call` on it, but not much beyond that. For one thing, it includes things like class constructors which are not callable (not without the `new` keyword), but TypeScript will still let you call it, but the return value is hardcoded to `any`. At the same time though, it won't let you assign a type of `Function` to a signature type (e.g. `(...args: any[]) => any)`) without an explicit cast. So generally, `Function` probably doesn't do what you want, has massive footgun around type safety when called, and should be avoided in favor of a suitable signature type, hence the eslint rule forbidding it. Notably, depending on the position, this is often one of the few cases where you legitimately have to use `any` over `unknown` (for the parameters and/or the return type), or else the variance/ subtyping may not work out the way you want. I think there are possibly pre-exisitng issues regarding this in the files I touched, but in the interest of keeping the PR focused and not leaching changes into public API, I did not address those in this commit. Ref open-telemetry#5365
9 tasks
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 27, 2025
``` /home/runner/work/opentelemetry-js/opentelemetry-js/packages/opentelemetry-context-async-hooks/src/AbstractAsyncHooksContextManager.ts 72:35 warning Don't use `Function` as a type @typescript-eslint/ban-types 134:60 warning Don't use `Function` as a type @typescript-eslint/ban-types 152:64 warning Don't use `Function` as a type @typescript-eslint/ban-types 176:15 warning Don't use `Function` as a type @typescript-eslint/ban-types /home/runner/work/opentelemetry-js/opentelemetry-js/packages/opentelemetry-context-async-hooks/src/AsyncHooksContextManager.ts 49:22 warning Forbidden non-null assertion @typescript-eslint/no-non-null-assertion ``` The first set of warnings are more involved, the second one just didn't turn out to be necessary at all and the non-null assertion (the `!` postfix operator) can simply be removed. Explaination: When using `typeof foo === 'function'` TypeScript narrows the type of `foo` to `Function`, so it may be tempting to use `Function` in signatures when you want to accept any callable function. However, this is not quite what `Function` means. `Function` as a type in TypeScript has inherited a fair bit of historical baggage and behaves strangely. For the most part, it only guarentee that it has the `Function` prototype, so has things like `.name`, `.bind` and `.call` on it, but not much beyond that. For one thing, it includes things like class constructors which are not callable (not without the `new` keyword), but TypeScript will still let you call it, but the return value is hardcoded to `any`. At the same time though, it won't let you assign a type of `Function` to a signature type (e.g. `(...args: any[]) => any)`) without an explicit cast. So generally, `Function` probably doesn't do what you want, has massive footgun around type safety when called, and should be avoided in favor of a suitable signature type, hence the eslint rule forbidding it. Notably, depending on the position, this is often one of the few cases where you legitimately have to use `any` over `unknown` (for the parameters and/or the return type), or else the variance/ subtyping may not work out the way you want. I think there are possibly pre-exisitng issues regarding this in the files I touched, but in the interest of keeping the PR focused and not leaching changes into public API, I did not address those in this commit. Ref open-telemetry#5365
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 27, 2025
```
/home/runner/work/opentelemetry-js/opentelemetry-js/packages/opentelemetry-core/src/utils/lodash.merge.ts
44:24 warning Don't use `Function` as a type @typescript-eslint/ban-types
```
This could have been addressed in various ways with no change to
the code, but upon further inspection, we can probably shed some
weight here.
This code was inherited from lodash, which defines:
```js
const getPrototype = overArg(Object.getPrototypeOf, Object);
```
`overArg` allows each argument of the passed in function to be
transformed by a mapper function, in this case, effective it
amounts to:
```js
const getPrototypeOf = Object.getPrototype;
function getPrototype(value) {
return getPrototypeOf(Object(value));
}
```
So at minimum, we can just inline _that_ and get rid of
`overArg`, since that's the only place we use that function.
The intent here appears to be making a "safe" version of
`Object.getPrototypeOf` that can be called with primitive
values.
For example, `Object.getPrototypeOf(null)` would throw, but
`Object.getPrototypeOf(Object(null))` wouldn't, and same for
`1`, `true`, etc.
Essentially, `Object(value)` will box the primitive value and
the function will return the prototype of the boxed primitive
object.
I suppose this is useful in the rest of the lodash codebase.
However, it's unclear that it is necessary in our stripped
down version.
`getPrototype` is only used in `isPlainObject`. The first
thing that function does is reject any non-object values
which would necessarily exclude all primitive values.
So I am pretty sure the `Object()` does nothing for us and
we can just call `Object.getPrototypeOf` directly and shed
all the extra stuff.
Ref open-telemetry#5365
8 tasks
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 27, 2025
``` /home/runner/work/opentelemetry-js/opentelemetry-js/packages/opentelemetry-resources/src/utils.ts 17:39 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any ``` Ref open-telemetry#5365
8 tasks
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 27, 2025
``` /home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-browser-detector/src/BrowserDetector.ts 66:66 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any ``` Ref open-telemetry#5365
8 tasks
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 28, 2025
``` /home/runner/work/opentelemetry-js/opentelemetry-js/packages/sdk-metrics/src/state/DeltaMetricProcessor.ts 96:32 warning Forbidden non-null assertion @typescript-eslint/no-non-null-assertion ``` The assertion was intentional, base on the `.has()` check with the same inputs immediately prior. The surrounding code has the same patterns and used the eslint magic comment to disable the warning, this one was just missed. Ref open-telemetry#5365
8 tasks
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 28, 2025
``` /home/runner/work/opentelemetry-js/opentelemetry-js/packages/opentelemetry-sdk-trace-node/test/registration.test.ts 30:61 warning Don't use `Function` as a type @typescript-eslint/ban-types ``` Here we are looking for a `AnyConstructor` type, and `Function` is a close enough approximation that exists in the standard library. The potential foot gun of _calling_ the function resulting in an `any` value still exists, but this is a small function used in tests only, so that's probably acceptable. Ref open-telemetry#5365
8 tasks
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 28, 2025
``` /home/runner/work/opentelemetry-js/opentelemetry-js/packages/opentelemetry-sdk-trace-web/src/utils.ts 52:14 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any ``` The eslint comment wasn't working because it was placed on the wrong line. However, it also wasn't necessary anymore – `keyof any` was used to obtain the union of all possible object key types – `string | number | symbol`. TypeScript now provides an alias for this in the standard libaray that gives exactly the same result. Also fixed what appears to be a TypeError on an un-annotated `let` binding, not sure why it wasn't caught by the type checker, maybe it was just missed on the version of TS we are using. Ref open-telemetry#5365
9 tasks
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 28, 2025
``` /home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-instrumentation/src/types.ts 140:24 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any 140:68 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any 146:24 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any ``` The annotations weren't working because they were placed on the wrong lines. Ref open-telemetry#5365
8 tasks
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 29, 2025
Addresses the following deprecation/eslint warning: ``` /home/runner/work/opentelemetry-js/opentelemetry-js/packages/opentelemetry-exporter-zipkin/src/platform/node/util.ts 34:19 warning 'url.parse' was deprecated since v11.0.0. Use 'url.URL' constructor instead node/no-deprecated-api ``` `http(s).request` now takes a URL (the global one, not `url.URL`) argument separate from the options bag, so we should be able to make this change compatibly, outside of some weird edge cases that are probably not applicable here. It is worth noting in the CHANGELOG since this used to trigger a Node runtime deprecation warning with `--pending-deprecation`. Ref open-telemetry#5365
8 tasks
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 29, 2025
Addresses the following deprecation/eslint warning: ``` /home/runner/work/opentelemetry-js/opentelemetry-js/packages/opentelemetry-exporter-zipkin/src/platform/node/util.ts 34:19 warning 'url.parse' was deprecated since v11.0.0. Use 'url.URL' constructor instead node/no-deprecated-api ``` `http(s).request` now takes a URL (the global one, not `url.URL`) argument separate from the options bag, so we should be able to make this change compatibly, outside of some weird edge cases that are probably not applicable here. It is worth noting in the CHANGELOG since this used to trigger a Node runtime deprecation warning with `--pending-deprecation`. Ref open-telemetry#5365
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 29, 2025
```
/home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/otlp-exporter-base/src/transport/http-exporter-transport.ts
55:9 warning Forbidden non-null assertion @typescript-eslint/no-non-null-assertion
/home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/otlp-exporter-base/src/transport/http-transport-utils.ts
95:35 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any
```
The first was that we know we already checked and initialized a
related variable, but TypeScript has no idea they are related. We
can better communicate that, both to TypeScript and to humans, by
groupping the related variables into a single one, that is either
set or not.
The second was a little strange, and I don't really see what the
`any` is doing there. `res.on('end')` is already typed to have an
`Error` type on the callback argument (maybe that wasn't always the
case?). If we trust the type, then there is nothing more we need to
do. If we don't trust the type, widening it to `any` without other
runtime checks isn't going to do anything for us either. So I just
removed it and everything was still fine.
Ref open-telemetry#5365
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 29, 2025
Fix the following eslint warnings:
```
/home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/otlp-exporter-base/src/transport/http-exporter-transport.ts
55:9 warning Forbidden non-null assertion @typescript-eslint/no-non-null-assertion
/home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/otlp-exporter-base/src/transport/http-transport-utils.ts
95:35 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any
```
The first was that we know we already checked and initialized a
related variable, but TypeScript has no idea they are related. We
can better communicate that, both to TypeScript and to humans, by
groupping the related variables into a single one, that is either
set or not.
The second was a little strange, and I don't really see what the
`any` is doing there. `res.on('end')` is already typed to have an
`Error` type on the callback argument (maybe that wasn't always the
case?). If we trust the type, then there is nothing more we need to
do. If we don't trust the type, widening it to `any` without other
runtime checks isn't going to do anything for us either. So I just
removed it and everything was still fine.
Ref open-telemetry#5365
8 tasks
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 29, 2025
Fix the following eslint warnings: ``` /home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-instrumentation-http/src/http.ts 236:41 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any 251:31 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any 252:31 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any 283:41 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any 298:31 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any 299:31 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any ``` Extracted a utility function with more precise types for the job. Ref open-telemetry#5365
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 29, 2025
Fix the following eslint warnings: ``` /home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-instrumentation-http/src/http.ts 508:17 warning Forbidden non-null assertion @typescript-eslint/no-non-null-assertion 931:11 warning Forbidden non-null assertion @typescript-eslint/no-non-null-assertion 1032:13 warning Forbidden non-null assertion @typescript-eslint/no-non-null-assertion 1043:13 warning Forbidden non-null assertion @typescript-eslint/no-non-null-assertion ``` Because the expression we check is indirected through the a method call (`getConfig()`), TypeScript cannot assume the value would be the same across the two calls. By extracting the value and checking for that, TypeScript can narrow the type correctly and we can avoid the non-null assertion. Ref open-telemetry#5365
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 29, 2025
``` /home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-instrumentation-http/src/http.ts 1051:15 warning Don't use `Function` as a type @typescript-eslint/ban-types ``` Be explict about what type of functions we are expecting here. Ref open-telemetry#5365
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 29, 2025
``` /home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-package.test.ts 86:27 warning 'url.parse' was deprecated since v11.0.0. Use 'url.URL' constructor instead node/no-deprecated-api /home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-instrumentation-http/test/functionals/https-package.test.ts 86:27 warning 'url.parse' was deprecated since v11.0.0. Use 'url.URL' constructor instead node/no-deprecated-api /home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts 81:25 warning 'url.parse' was deprecated since v11.0.0. Use 'url.URL' constructor instead node/no-deprecated-api 156:43 warning 'url.parse' was deprecated since v11.0.0. Use 'url.URL' constructor instead node/no-deprecated-api 161:43 warning 'url.parse' was deprecated since v11.0.0. Use 'url.URL' constructor instead node/no-deprecated-api 213:35 warning 'url.parse' was deprecated since v11.0.0. Use 'url.URL' constructor instead node/no-deprecated-api /home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-instrumentation-http/test/integrations/http-enable.test.ts 300:9 warning 'url.parse' was deprecated since v11.0.0. Use 'url.URL' constructor instead node/no-deprecated-api /home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-instrumentation-http/test/integrations/https-enable.test.ts 274:9 warning 'url.parse' was deprecated since v11.0.0. Use 'url.URL' constructor instead node/no-deprecated-api ``` Generally speaking, `new URL()` is not a direct replacement for the deprecated `url.parse()`, so this type of change requires careful considerations. However, in this instance, these are all found in test code, which cuts out a lot of the typically associated issues, and the tests passing after the change is a good indication for correctness. Ref open-telemetry#5365
10 tasks
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 29, 2025
Fixes the following eslint warning in exporter-metrics-otlp-http: ``` /home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/OTLPMetricExporterBase.ts 120:30 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any ``` The unused parameter can be explicitly typed as `InstrumentType`, but since this default selector doesn't actually care about it, it can also just be elided. By adding an explict return type, if someone in the future added the wrong parameter to the default selector function, TypeScript would reject it. Ref open-telemetry#5365
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 29, 2025
Fixes the following eslint warning in exporter-metrics-otlp-http: ``` /home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/OTLPMetricExporterBase.ts 120:30 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any ``` The unused parameter can be explicitly typed as `InstrumentType`, but since this default selector doesn't actually care about it, it can also just be elided. By adding an explicit return type, if someone in the future added the wrong parameter to the default selector function, TypeScript would reject it. Ref open-telemetry#5365
This was referenced Jan 29, 2025
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 30, 2025
``` /home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/exporter-trace-otlp-http/src/index.ts 20:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax /home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/exporter-trace-otlp-http/src/platform/browser/index.ts 20:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax /home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/exporter-trace-otlp-http/src/platform/index.ts 20:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax /home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/exporter-trace-otlp-http/src/platform/node/index.ts 20:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax ``` The comment say we should do this before the next "major" (minor since we are pre-1.0?) version, which means this is the right time to do it. However, I'm not sure if there is a real need/reason for that. As far as I can tell, this is still exporting exactly the same items and shouldn't cause any breakages? Ref open-telemetry#5365
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 30, 2025
``` /home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/exporter-trace-otlp-grpc/src/index.ts 20:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax ``` The comment say we should do this before the next "major" (minor since we are pre-1.0?) version, which means this is the right time to do it. However, I'm not sure if there is a real need/reason for that. As far as I can tell, this is still exporting exactly the same items and shouldn't cause any breakages? Ref open-telemetry#5365
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 30, 2025
``` /home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/src/index.ts 20:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax ``` The comment say we should do this before the next "major" (minor since we are pre-1.0?) version, which means this is the right time to do it. However, I'm not sure if there is a real need/reason for that. As far as I can tell, this is still exporting exactly the same items and shouldn't cause any breakages? Ref open-telemetry#5365
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 30, 2025
``` /home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/src/index.ts 20:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax ``` The comment say we should do this before the next "major" (minor since we are pre-1.0?) version, which means this is the right time to do it. However, I'm not sure if there is a real need/reason for that. As far as I can tell, this is still exporting exactly the same items and shouldn't cause any breakages? Ref open-telemetry#5365
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 30, 2025
``` /home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/exporter-logs-otlp-grpc/src/index.ts 20:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax ``` The comment say we should do this before the next "major" (minor since we are pre-1.0?) version, which means this is the right time to do it. However, I'm not sure if there is a real need/reason for that. As far as I can tell, this is still exporting exactly the same items and shouldn't cause any breakages? Ref open-telemetry#5365
8 tasks
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 30, 2025
``` /home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-sdk-node/src/index.ts 20:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax 21:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax 22:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax 23:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax 24:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax 25:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax 26:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax 27:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax ``` This feels a bit different than the other ones, as this is a meta- package, and these are wildcard exports to re-export all the public exports from the upstream packages into individual _namesapces_. There is no risk of accidentally exporting something that wasn't meant to be public, and it would be a huge pain to enumerate things from each of the upstream packages and keep things in-sync. Ref open-telemetry#5365
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 30, 2025
``` /home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-sdk-node/src/index.ts 20:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax 21:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax 22:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax 23:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax 24:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax 25:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax 26:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax 27:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax ``` This feels a bit different than the other ones, as this is a meta- package, and these are wildcard exports to re-export all the public exports from the upstream packages into individual _namespaces_. There is no risk of accidentally exporting something that wasn't meant to be public, and it would be a huge pain to enumerate things from each of the upstream packages and keep things in-sync. Ref open-telemetry#5365
8 tasks
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 30, 2025
``` /home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-sdk-node/src/index.ts 20:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax 21:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax 22:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax 23:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax 24:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax 25:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax 26:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax 27:1 warning Using 'ExportAllDeclaration' is not allowed no-restricted-syntax ``` This feels a bit different than the other ones, as this is a meta- package, and these are wildcard exports to re-export all the public exports from the upstream packages into individual _namespaces_. There is no risk of accidentally exporting something that wasn't meant to be public, and it would be a huge pain to enumerate things from each of the upstream packages and keep things in-sync. Ref open-telemetry#5365
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 30, 2025
``` /home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-instrumentation-fetch/src/utils.ts 137:16 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any 138:21 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any ``` This can be avoided by re-ordering the branches so that TypeScript can narrow the type of the value progressively. Ref open-telemetry#5365
8 tasks
chancancode
added a commit
to tildeio/opentelemetry-js
that referenced
this issue
Jan 30, 2025
``` /home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-instrumentation-xml-http-request/src/types.ts 66:28 warning Don't use `Function` as a type @typescript-eslint/ban-types /home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-instrumentation-xml-http-request/src/utils.ts 44:16 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any 45:21 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any /home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-instrumentation-xml-http-request/test/xhr.test.ts 69:22 warning Don't use `Function` as a type @typescript-eslint/ban-types 104:22 warning Don't use `Function` as a type @typescript-eslint/ban-types ``` The middle ones are the same as open-telemetry#5401, the rest are replacing `Function` with a more specific call signature, and they are either in tests or private API, so no breakages expected. Ref open-telemetry#5365
8 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
As discussed in the SIG a few weeks ago, I intended to cleanup the eslint warnings in the repository. Here is a laundry list for me to keep track of progress.
refactor(api): fix eslint warnings #5366
feat(api)!: Move
TextMapPropagator<Carrier>generic parameter to interface methods #5368docs(api): clarify
TextMapPropagatorAPI requirements #5370refactor(context-zone-peer-dep): fix eslint warnings #5371
refactor(context-async-hooks): fix eslint warnings #5381
refactor(resources): fix eslint warning #5383
refactor(otlp-exporter-base): fix eslint warnigns #5391
refactor(sdk-node): fix eslint warning #5400
refactor(sdk-trace-base): fix eslint warnings #5385
refactor(instrumentation-http): fix eslint warnings #5394
refactor(instrumentation-xhr): fix eslint warnings #5402
refactor(core): fix eslint warning #5382
refactor(browser-detector): fix eslint warning #5384
refactor(sdk-metrics): fix eslint warning #5386
refactor(sdk-trace-node): fix eslint warning #5387
refactor(sdk-trace-web): fix eslint warning #5388
refactor(instrumentation): fix eslint warnings #5389
fix(exporter-zipkin): remove deprecated
url.parseusage #5390refactor(exporter-metrics-otlp-http): fix eslint warning #5396
refactor(exporter-trace-otlp-http): fix eslint warnings #5398
refactor(exporter-*): remove
export * from ...#5399refactor(exporter-*): remove
export * from ...#5399refactor(exporter-*): remove
export * from ...#5399refactor(exporter-*): remove
export * from ...#5399refactor(exporter-*): remove
export * from ...#5399refactor(instrumentation-fetch): fix eslint warnings #5401
The text was updated successfully, but these errors were encountered: