Open nick4598 opened 1 month ago
This was added because this library relied (and I imagine still does) on internal (sometimes native addon) APIs.
You can get much harder to understand errors without this probably.
Yes it's a false positive in a lot of cases, especially when you opt to use "-dev" versions.
Yes you're right that semver.satisfies never returns true when comparing differently tagged versions.
You can probably clear the dev version mostly safely considering how it's used, but it is incorrect in some real cases.
But why is the user swallowing the error such that it is so hard to find in the first place?
Any logger they're using should be printing uncaught errors, which is the default behavior anyway.
If the user didn't understand the error once found, maybe that also needs an improvement. (E.g. mention that -dev versions aren't considered (unless you change that))
You're right the library does still rely on some internal APIs, there has been some movement in the itwinjs core repo in the direction of making it harder to use internal APIs.
Do you happen to have any examples in mind of how clearing the dev version is incorrect in some real cases?
@ashar-bentley any ideas regarding "But why is the user swallowing the error such that it is so hard to find in the first place?"
Recently I was approached with a question about why a line of code like
import {IModelTransformer} from @itwin/imodel-transformer
was enough to crash a program on startup I didn't know the answer unfortunately, but eventually that person was able to determine that the crash was coming from the check for matching peer deps.This person had to find the error in the debugger, which seems unfortunate and I'd rather them be able to see this error immediately. I'm not sure if this is possible or not though.
Action Items