refactor(effects): prepare for TS 3.5 #2191
Conversation
|
Preview docs changes for 3151941 at https://previews.ngrx.io/pr2191-3151941/ |
| @@ -61,7 +61,9 @@ export function createEffect< | |||
| return effect; | |||
| } | |||
|
|
|||
| export function getCreateEffectMetadata<T>(instance: T): EffectMetadata<T>[] { | |||
| export function getCreateEffectMetadata< | |||
| T extends { [props in keyof T]: Object } | |||
There was a problem hiding this comment.
May be T extends Record<keyof T, Object> would be better?
There was a problem hiding this comment.
Yes, it's the same thing. Don't think we used Record at the codebase yet.
| @@ -21,7 +22,7 @@ export function Effect<T>({ | |||
| resubscribeOnError, | |||
| }; | |||
| setEffectMetadataEntries<T>(target, [metadata]); | |||
| } as (target: {}, propertyName: string | symbol) => void; | |||
| }; | |||
| @@ -46,7 +46,7 @@ function getEffectName({ | |||
| }: EffectNotification) { | |||
| const isMethod = typeof sourceInstance[propertyName] === 'function'; | |||
|
|
|||
| return `"${sourceName}.${propertyName}${isMethod ? '()' : ''}"`; | |||
| return `"${sourceName}.${String(propertyName)}${isMethod ? '()' : ''}"`; | |||
There was a problem hiding this comment.
Why this change required? I thought Typescript knew that propertyName will be transformer to string under the hood
There was a problem hiding this comment.
Good question @L2jLiga ! I'm bringing the propertyName back to the actual allowed property types, which are string | number | Symbol.
Symbol cannot be implicitly converted to string, and needs some help.
I probably should rename propertyName to the propertyKey to be more descriptive. I'll leave it out for later. Thanks for asking!
|
I think it would be nice to adapt to TS 3.6 as far as Angular 9 rc0 published |
I haven't check if 3.6 brings any problem yet. Let's take it one-by-one. |
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
TS 3.5 becomes stricter with type, and generic
Tdoesn't haveconstructororhasOwnPropertymethods.for example, see microsoft/TypeScript#31380
What is the current behavior?
Closes #
What is the new behavior?
Fix issues that would come up with TS 3.5 upgrade
Does this PR introduce a breaking change?
Other information