feat(build): add more TypeScript "strict" checks #2704
Conversation
bajtos
added
the
Internal Tooling
packages/context/src/__tests__/acceptance/class-level-bindings.acceptance.ts
Outdated
Show resolved
Hide resolved
bajtos
force-pushed
the
feat/tsc-strict-checks
branch
from
82cdcd6 to
234ec10
Compare
|
Now #2711 is landed, please rebase against latest master. |
bajtos
force-pushed
the
feat/tsc-strict-checks
branch
from
234ec10 to
af016a4
Compare
Done. I have also changed the way how I address the problem with @raymondfeng LGTY now? |
bajtos
force-pushed
the
feat/tsc-strict-checks
branch
2 times, most recently
from
bfffe47 to
a18b41b
Compare
| * We learned about this problem after enabling TypeScript's `strictFunctionTypes` | ||
| * check, but decided to preserve `ValueType` argument for backwards compatibility. | ||
| * | ||
| * TODO(semver-major): remove ValueType template argument. |
There was a problem hiding this comment.
See #2728. The ValueType is used internally with a default. Applications or extensions may define their own filter functions and mostly won't cast it to BindingFilter<...>. I don't think we have to release a new major.
|
Is there a possibility that this change will break existing applications? |
packages/context/src/context-view.ts
Outdated
| @@ -161,10 +161,10 @@ export class ContextView<T = unknown> extends EventEmitter | |||
| */ | |||
| export function createViewGetter<T = unknown>( | |||
| ctx: Context, | |||
| bindingFilter: BindingFilter<T>, | |||
| bindingFilter: BindingFilter, | |||
There was a problem hiding this comment.
Before this change, it was possible to infer T in createViewGetter from bindingFilter type.
const filter: BindingFilter<MyValue> = b => true;
const getter = createViewGetter(ctx, filter);
// ^^^ getter is of type Getter<MyValue[]>With this change in place, users have to explicitly specify the getter type, otherwise they end up with unknown.
const getter = createViewGetter(ctx, filter);
// ^^^ getter is of type Getter<unknown[]>
const getter = createViewGetter<MyValue>(ctx, filter);
// ^^^ getter is of type Getter<MyValue[]>
packages/context/src/context-view.ts
Outdated
| session?: ResolutionSession, | ||
| ): Getter<T[]> { | ||
| const view = new ContextView(ctx, bindingFilter); | ||
| const view = new ContextView<T>(ctx, bindingFilter); |
There was a problem hiding this comment.
Similarly here. Before this change, it was possible to infer T of ContextView from bindingFilter type. After this change, new ContextView(...) always treats T as unknown.
Good question, see my two comments above. |
|
Also any project using I don't consider this as a change that would require semver-major, because this situation happens regularly with new semver-minor releases of TypeScript. TypeScript releases a new version, e.g. 3.4, consumers upgrade to this new version (because |
|
I have extracted the first two commits to get them landed faster: #2733 |
bajtos
force-pushed
the
feat/tsc-strict-checks
branch
2 times, most recently
from
ed788bf to
e37e5f4
Compare
Enable all "strict" checks with the exception of `strictPropertyInitialization` which would require significant changes. The following additional checks are added: - noImplicitThis - alwaysStrict - strictFunctionTypes In the future, any checks added to TypeScript "strict" mode will be automatically enabled for LoopBack projects too.
bajtos
force-pushed
the
feat/tsc-strict-checks
branch
from
e37e5f4 to
70af431
Compare
|
@raymondfeng I have rebased the pull request on top of the latest |
|
@bajtos I landed for you to reduce the backlog of PRs. |
Enable all "strict" checks with the exception of
strictPropertyInitializationwhich would require significant changes.The following additional checks are added:
In the future, any checks added to TypeScript "strict" mode will be automatically enabled for LoopBack projects too.
See also TypeScript Compiler Options
Checklist
👉 Read and sign the CLA (Contributor License Agreement) 👈
npm testpasses on your machinepackages/cliwere updatedexamples/*were updated👉 Check out how to submit a PR 👈