Skip to content

Commit

Permalink
refactor(decorators): update decorator types @W-16373548@ (#4429)
Browse files Browse the repository at this point in the history
* chore: hushsky

* feat(wire): ensure prop type matches adapter value

* fix(wire): account for reactive prop replacement

* feat(wire): drop experimental decorator signature

* refactor(api): only support modern decorators

also, only allow @api on LightningElement classes

* refactor(track): only support modern decorators

also, only allow on LightningElements

* refactor(wire): only support modern decorators

also, only allow on LightningElements

* refactor(integration-types): remove tests for experimental decorators

* docs: add comment explaining assertions

* refactor(playground): remove experimental decorator support

* refactor(integration-types): split decorator tests into multiple files

one file per decorator

* refactor(track): make function signature work as decorator and non-decorator

* refactor(wire): allow values to be undefined

* test(wire): expand type validations

* docs: simplify comment

* test(wire): expand type validations

* feat(wire): properly type reactive values

* test(wire): expand type validations

* test(decorators): validate type error when used on non-components

* refactor(wire): method param is never explicit undefined

* test(wire): split type validations into separate classes

* refactor(wire): @wire cannot be used on getters/setters

* test(wire): validate that nested props are not reactive

* fix(test): use correct context type

* docs: clarify comment

* revert(track): check arguments instead of undefined

* test(wire): validate wire doesn't work on non-component with superclass

* fix(wire): make wire work with adapters typed as `any`

* test(types): assert decorators don't work on non-LightningElement classes

* refactor(wire): remove type error if extending a non-component

TypeScript can't differentiate between a class without a superclass
and a class that extends `any`. Because extending `any` is probably
a fairly common use case (e.g. base components without a type stub),
we need to allow it, at the cost of also allowing non-extended classes.

* feat(wire): update signature to allow decorating getters/setters

* docs(wire): add comments to decorator type

* refactor(wire): update type def to always split on "."

runtime behavior is to use "." as a delimiter, never as part of the property key

* test(wire): update type validations for prop/method decorators

* test(wire): add type validations for decorated getters

* test(wire): add type validations for decorated setters

* feat(wire): allow undefined in wired getter

* refactor(decorators): relax type constraint restricting to LightningElement

Components could extend a superclass typed as `any`

* chore(decorators): remove useless generic type param

* docs(test): clarify use of nested.prop

* chore: prettier
  • Loading branch information
wjhsf authored Sep 6, 2024
1 parent 1a775cc commit d79c246
Show file tree
Hide file tree
Showing 12 changed files with 559 additions and 53 deletions.
4 changes: 2 additions & 2 deletions packages/@lwc/engine-core/src/framework/decorators/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ import { assert, isFunction, isNull, toString } from '@lwc/shared';
import { logError } from '../../shared/logger';
import { isInvokingRender, isBeingConstructed } from '../invoker';
import { componentValueObserved, componentValueMutated } from '../mutation-tracker';
import { LightningElement } from '../base-lightning-element';
import { getAssociatedVM } from '../vm';
import { isUpdatingTemplate, getVMBeingRendered } from '../template';
import type { LightningElement } from '../base-lightning-element';

/**
* The `@api` decorator marks public fields and public methods in
Expand All @@ -21,7 +21,7 @@ export default function api(
// eslint-disable-next-line @typescript-eslint/no-unused-vars
value: unknown,
// eslint-disable-next-line @typescript-eslint/no-unused-vars
context: ClassMemberDecoratorContext | string | symbol
context: ClassMemberDecoratorContext
): void {
if (process.env.NODE_ENV !== 'production') {
assert.fail(`@api decorator can only be used as a decorator function.`);
Expand Down
12 changes: 7 additions & 5 deletions packages/@lwc/engine-core/src/framework/decorators/track.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,23 @@ import { componentValueObserved } from '../mutation-tracker';
import { isInvokingRender } from '../invoker';
import { getAssociatedVM } from '../vm';
import { getReactiveProxy } from '../membrane';
import { LightningElement } from '../base-lightning-element';
import { isUpdatingTemplate, getVMBeingRendered } from '../template';
import { updateComponentValue } from '../update-component-value';
import { logError } from '../../shared/logger';
import type { LightningElement } from '../base-lightning-element';

/**
* The `@track` decorator function marks field values as reactive in
* LWC Components. This function can also be invoked directly
* with any value to obtain the trackable version of the value.
*/
export default function track(target: undefined, context: ClassFieldDecoratorContext): void;
export default function track<T>(target: T, context?: never): T;
export default function track(
value: unknown,
context: ClassMemberDecoratorContext | string | symbol
): void;
export default function track<T>(target: T): T {
target: unknown,
// eslint-disable-next-line @typescript-eslint/no-unused-vars
context?: ClassFieldDecoratorContext
): unknown {
if (arguments.length === 1) {
return getReactiveProxy(target);
}
Expand Down
53 changes: 47 additions & 6 deletions packages/@lwc/engine-core/src/framework/decorators/wire.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,47 @@
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
*/
import { assert } from '@lwc/shared';
import { LightningElement } from '../base-lightning-element';
import { componentValueObserved } from '../mutation-tracker';
import { getAssociatedVM } from '../vm';
import { WireAdapterConstructor } from '../wiring';
import { updateComponentValue } from '../update-component-value';
import type { LightningElement } from '../base-lightning-element';
import type {
ConfigValue,
ContextValue,
ReplaceReactiveValues,
WireAdapterConstructor,
} from '../wiring';

/**
* The decorator returned by `@wire()`; not the `wire` function.
*
* For TypeScript users:
* - If you are seeing an unclear error message, ensure that both the type of the decorated prop and
* the config used match the types expected by the wire adapter.
* - String literal types in the config are resolved to the corresponding prop on the component.
* For example, a component with `id = 555` and `@wire(getBook, {id: "$id"} as const) book` will
* have `"$id"` resolve to type `number`.
*/
interface WireDecorator<Value, Class> {
(
target: unknown,
context: // A wired prop doesn't have any data on creation, so we must allow `undefined`
| ClassFieldDecoratorContext<Class, Value | undefined>
| ClassMethodDecoratorContext<
Class,
// When a wire adapter is typed as `WireAdapterConstructor`, then this `Value`
// generic is inferred as the value used by the adapter for all decorator contexts
// (field/method/getter/setter). But when the adapter is typed as `any`, then
// decorated methods have `Value` inferred as the full method. (I'm not sure why.)
// This conditional checks `Value` so that we get the correct decorator context.
Value extends (value: any) => any ? Value : (this: Class, value: Value) => void
>
// The implementation of a wired getter/setter is ignored; they are treated identically
// to wired props. Wired props don't have data on creation, so we must allow `undefined`
| ClassGetterDecoratorContext<Class, Value | undefined>
| ClassSetterDecoratorContext<Class, Value>
): void;
}

/**
* Decorator factory to wire a property or method to a wire adapter data source.
Expand All @@ -22,12 +58,17 @@ import { updateComponentValue } from '../update-component-value';
* \@wire(getBook, { id: '$bookId'}) book;
* }
*/
export default function wire(
export default function wire<
ReactiveConfig extends ConfigValue = ConfigValue,
Value = any,
Context extends ContextValue = ContextValue,
Class = LightningElement,
>(
// eslint-disable-next-line @typescript-eslint/no-unused-vars
adapter: WireAdapterConstructor,
adapter: WireAdapterConstructor<ReplaceReactiveValues<ReactiveConfig, Class>, Value, Context>,
// eslint-disable-next-line @typescript-eslint/no-unused-vars
config?: Record<string, any>
): (value: unknown, context: ClassMemberDecoratorContext | string | symbol) => void {
config?: ReactiveConfig
): WireDecorator<Value, Class> {
if (process.env.NODE_ENV !== 'production') {
assert.fail('@wire(adapter, config?) may only be used as a decorator.');
}
Expand Down
1 change: 1 addition & 0 deletions packages/@lwc/engine-core/src/framework/wiring/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export {
ContextProvider,
ContextValue,
DataCallback,
ReplaceReactiveValues,
WireAdapter,
WireAdapterConstructor,
WireAdapterSchemaValue,
Expand Down
36 changes: 36 additions & 0 deletions packages/@lwc/engine-core/src/framework/wiring/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,3 +87,39 @@ export type RegisterContextProviderFn = (
adapterContextToken: string,
onContextSubscription: WireContextSubscriptionCallback
) => void;

/** Resolves a property chain to the corresponding value on the target type. */
type ResolveReactiveValue<
/** The object to search for properties; initially the component. */
Target,
/** A string representing a chain of of property keys, e.g. "data.user.name". */
Keys extends string,
> = Keys extends `${infer FirstKey}.${infer Rest}`
? // If the string is "a.b.c", check if "a" is a prop on the target object
FirstKey extends keyof Target
? // If "a" exists on the target, check `target["a"]` for "b.c"
ResolveReactiveValue<Target[FirstKey], Rest>
: undefined
: // The string has no ".", use the full string as the key (e.g. we've reached "c" in "a.b.c")
Keys extends keyof Target
? Target[Keys]
: undefined;

/**
* Detects if the `Value` type is a property chain starting with "$". If so, it resolves the
* properties to the corresponding value on the target type.
*/
type ResolveValueIfReactive<Value, Target> = Value extends string
? string extends Value // `Value` is type `string`
? // Workaround for not being able to enforce `as const` assertions -- we don't know if this
// is a true string value (e.g. `@wire(adapter, {val: 'str'})`) or if it's a reactive prop
// (e.g. `@wire(adapter, {val: '$number'})`), so we have to go broad to avoid type errors.
any
: Value extends `$${infer Keys}` // String literal starting with "$", e.g. `$prop`
? ResolveReactiveValue<Target, Keys>
: Value // String literal *not* starting with "$", e.g. `"hello world"`
: Value; // non-string type

export type ReplaceReactiveValues<Config extends ConfigValue, Component> = {
[K in keyof Config]: ResolveValueIfReactive<Config[K], Component>;
};
3 changes: 1 addition & 2 deletions packages/@lwc/integration-types/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
"description": "Type validation for LWC packages",
"type": "module",
"scripts": {
"test": "node ./scripts/update-paths.js --check && tsc && yarn run test:experimental-decorators",
"test:experimental-decorators": "tsc -p tsconfig.experimental-decorators.json",
"test": "node ./scripts/update-paths.js --check && tsc",
"playground": "rollup -c src/playground/rollup.config.js --watch"
},
"dependencies": {
Expand Down
25 changes: 0 additions & 25 deletions packages/@lwc/integration-types/src/decorators.ts

This file was deleted.

22 changes: 22 additions & 0 deletions packages/@lwc/integration-types/src/decorators/api.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*
* Copyright (c) 2024, Salesforce, Inc.
* All rights reserved.
* SPDX-License-Identifier: MIT
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
*/

import { LightningElement, api } from 'lwc';

// @ts-expect-error bare decorator cannot be used
api();

// @ts-expect-error decorator doesn't work on classes
@api
export default class Test extends LightningElement {
@api optionalProperty?: string;
@api propertyWithDefault = true;
@api nonNullAssertedProperty!: object;
@api method() {}
@api getter(): undefined {}
@api setter(_: string) {}
}
31 changes: 31 additions & 0 deletions packages/@lwc/integration-types/src/decorators/track.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* Copyright (c) 2024, Salesforce, Inc.
* All rights reserved.
* SPDX-License-Identifier: MIT
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
*/

import { LightningElement, track } from 'lwc';

// This is okay! track has a non-decorator signature
track(123);
// This is okay because we treat implicit and explicit `undefined` identically
track(123, undefined);
// @ts-expect-error wrong number of arguments
track();
// @ts-expect-error wrong number of arguments
track({}, {});

// @ts-expect-error doesn't work on classes
@track
export default class Test extends LightningElement {
@track optionalProperty?: string;
@track propertyWithDefault = true;
@track nonNullAssertedProperty!: object;
// @ts-expect-error cannot be used on methods
@track method() {}
// @ts-expect-error cannot be used on getters
@track getter(): undefined {}
// @ts-expect-error cannot be used on setters
@track setter(_: string) {}
}
Loading

0 comments on commit d79c246

Please sign in to comment.