Skip to content

Commit

Permalink
The refactor to the reference system attempted to squash/normalize th…
Browse files Browse the repository at this point in the history
…e difference between a `(mut)` reference (previously – "Invokable" reference, admittedly a very confusing name) and an "accessor" reference.

This was a mistake. The Ember usage that necessitated this feature requires that they are distinct. Specifically `{{action (mut this.someProp)}}` is explicitly a distinctly different usage than `{{action this.someProp}}`.
  • Loading branch information
chancancode committed Dec 14, 2023
1 parent 33bab34 commit 53031fc
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 27 deletions.
10 changes: 9 additions & 1 deletion packages/@glimmer/interfaces/lib/references.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,11 @@ export type ComputedCellType = 4;
* An accessor has both a user-space computation and a userspace update. Both are fallible.
*/
export type AccessorType = 5;
export type ConstantErrorType = 6;
/**
* An accessor that was explicitly marked using the toMut() API.
*/
export type MutableReferenceType = 6;
export type ConstantErrorType = 7;

export interface ReactiveTypes {
readonly MutableCell: MutableCellType;
Expand All @@ -39,6 +43,7 @@ export interface ReactiveTypes {
readonly ComputedCell: ReactiveComputedCell;
readonly Formula: FormulaType;
readonly Accessor: AccessorType;
readonly MutableReference: MutableReferenceType;
readonly ConstantError: ConstantErrorType;
}

Expand All @@ -49,6 +54,7 @@ export type ReactiveType =
| ComputedCellType
| FormulaType
| AccessorType
| MutableReferenceType
| ConstantErrorType;

declare const REFERENCE: unique symbol;
Expand Down Expand Up @@ -185,6 +191,7 @@ export type Reactive<T = unknown> =
| ReactiveComputedCell<T>
| ReactiveFormula<T>
| ReactiveAccessor<T>
| ReactiveMutableReference<T>
| ConstantReactiveError;

export type DeeplyConstantReactiveCell<T = unknown> = RawReactive<T, DeeplyConstantType>;
Expand All @@ -207,5 +214,6 @@ export type ReactiveCell<T = unknown> =
export type ReactiveComputedCell<T = unknown> = RawReactive<T, ComputedCellType>;
export type ReactiveFormula<T = unknown> = RawReactive<T, FormulaType>;
export type ReactiveAccessor<T = unknown> = RawReactive<T, AccessorType>;
export type ReactiveMutableReference<T = unknown> = RawReactive<T, MutableReferenceType>;

export type ReactiveResult<T> = Result<T, UserException>;
19 changes: 16 additions & 3 deletions packages/@glimmer/reference/lib/api/accessor.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import type {
AccessorType,
DefaultDescriptionFields,
DescriptionSpec,
MutableReferenceType,
Reactive,
ReactiveResult,
} from '@glimmer/interfaces';
Expand Down Expand Up @@ -31,11 +33,22 @@ export function ResultAccessor<T = unknown>(
get: () => ReactiveResult<T>;
set: (val: T) => ReactiveResult<void>;
},
description?: DescriptionSpec
description?: DescriptionSpec,
): Reactive<T> {
const { get, set } = options;
return InternalResultAccessor(options, description);
}

const internal = new InternalReactive<T>(ACCESSOR);
export function InternalResultAccessor<T = unknown>(
options: {
get: () => ReactiveResult<T>;
set: (val: T) => ReactiveResult<void>;
},
description?: DescriptionSpec,
type: AccessorType | MutableReferenceType = ACCESSOR,
): Reactive<T> {
const { get, set } = options;

const internal = new InternalReactive<T>(type);

internal.compute = () => setResult(internal, get());

Expand Down
11 changes: 1 addition & 10 deletions packages/@glimmer/reference/lib/api/cell.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,4 @@
import type {
DeeplyConstantReactiveCell,
DefaultDescriptionFields,
Described,
DescriptionSpec,
MutableReactiveCell,
ReactiveCell,
ReferenceDescription,
RETURN_TYPE,
} from '@glimmer/interfaces';
import type {DeeplyConstantReactiveCell, DefaultDescriptionFields, Described, DescriptionSpec, MutableReactiveCell, ReactiveCell, ReferenceDescription, RETURN_TYPE} from '@glimmer/interfaces';

This comment has been minimized.

Copy link
@NullVoxPopuli

NullVoxPopuli Dec 14, 2023

Contributor

needs pnpm linfix

import { devmode, setDescription, toValidatableDescription } from '@glimmer/util';
import { CONSTANT_TAG, consumeTag, createTag, dirtyTag } from '@glimmer/validator';

Expand Down
5 changes: 4 additions & 1 deletion packages/@glimmer/reference/lib/api/internal/reactive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type {
DevMode,
FormulaType,
MutableCellType,
MutableReferenceType,
Nullable,
Optional,
RawReactive,
Expand All @@ -29,7 +30,8 @@ export const DEEPLY_CONSTANT: DeeplyConstantType = 2;
export const FALLIBLE_FORMULA: FormulaType = 3;
export const COMPUTED_CELL: ComputedCellType = 4;
export const ACCESSOR: AccessorType = 5;
export const CONSTANT_ERROR: ConstantErrorType = 6;
export const MUTABLE_REF: MutableReferenceType = 6;
export const CONSTANT_ERROR: ConstantErrorType = 7;

export const REACTIVE_DESCRIPTIONS = [
'mutable',
Expand All @@ -38,6 +40,7 @@ export const REACTIVE_DESCRIPTIONS = [
'fallible formula',
'infallible formula',
'accessor',
'mut reference',

This comment has been minimized.

Copy link
@NullVoxPopuli

NullVoxPopuli Dec 14, 2023

Contributor

mut

'constant error',
];

Expand Down
27 changes: 20 additions & 7 deletions packages/@glimmer/reference/lib/api/mutability.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,38 @@
import type { Reactive, ReactiveFormula } from '@glimmer/interfaces';
import type { Reactive } from '@glimmer/interfaces';

import type { InternalReactive } from './internal/reactive';

import { ResultAccessor } from './accessor';
import { InternalResultAccessor } from './accessor';
import { unwrapReactive } from './core';
import { Formula } from './formula';
import { readInternalReactive, updateInternalReactive } from './internal/operations';
import { DEEPLY_CONSTANT, READONLY_CELL, REFERENCE } from './internal/reactive';
import { DEEPLY_CONSTANT, MUTABLE_REF, READONLY_CELL, REFERENCE } from './internal/reactive';
import { isMutRef, isUpdatableRef } from './predicates';

export function toReadonly<T>(reactive: Reactive<T>): ReactiveFormula<T> {
return Formula(() => unwrapReactive(reactive));
export function toReadonly<T>(reactive: Reactive<T>): Reactive<T> {
if (isMutRef(reactive) || isUpdatableRef(reactive)) {
return Formula(() => unwrapReactive(reactive));
} else {
return reactive;

This comment has been minimized.

Copy link
@NullVoxPopuli

NullVoxPopuli Dec 14, 2023

Contributor

is this implying that the input is already readonly?

are there only these 3 kinds? Mut, Updateable, and Readonly?

}
}

export function toMut<T>(maybeMut: Reactive<T>): Reactive<T> {
const reactive = maybeMut as InternalReactive;

return ResultAccessor({
if (isMutRef(maybeMut)) {
return maybeMut;
}

// TODO probably should assert that maybeMut is updatable

This comment has been minimized.

Copy link
@NullVoxPopuli

NullVoxPopuli Dec 14, 2023

Contributor

what's the difference between updatable and mut?

// Ember already has the same assertion

return InternalResultAccessor({
get: () => readInternalReactive(maybeMut as InternalReactive<T>),
set: (value: unknown) => updateInternalReactive(reactive, value),
});
}, undefined, MUTABLE_REF);
}

export function isConstant(reactive: Reactive) {
switch (reactive[REFERENCE]) {
case READONLY_CELL:
Expand Down
10 changes: 8 additions & 2 deletions packages/@glimmer/reference/lib/api/predicates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ import type {
Reactive,
ReactiveAccessor,
ReactiveFormula,
ReactiveMutableReference,
} from '@glimmer/interfaces';

import type { InternalReactive } from './internal/reactive';

import { ACCESSOR, CONSTANT_ERROR, FALLIBLE_FORMULA, REFERENCE } from './internal/reactive';
import { ACCESSOR, CONSTANT_ERROR, FALLIBLE_FORMULA, MUTABLE_REF, REFERENCE } from './internal/reactive';

export function isFallibleFormula<T>(_ref: Reactive<T>): _ref is ReactiveFormula<T> {
return _ref[REFERENCE] === FALLIBLE_FORMULA;
Expand All @@ -17,11 +18,16 @@ export function isAccessor<T>(_ref: Reactive<T>): _ref is ReactiveAccessor<T> {
return _ref[REFERENCE] === ACCESSOR;
}

export function isMutRef<T>(_ref: Reactive<T>): _ref is ReactiveMutableReference<T> {
return _ref[REFERENCE] === MUTABLE_REF;
}

export function isConstantError<T>(_ref: Reactive<T>): _ref is ConstantReactiveError {
return _ref[REFERENCE] === CONSTANT_ERROR;
}

export function isUpdatableRef(_ref: Reactive) {
const ref = _ref as InternalReactive;

return ref.update !== null;
return (isAccessor(_ref) || isMutRef(_ref)) && ref.update !== null;
}
6 changes: 3 additions & 3 deletions packages/@glimmer/runtime/lib/helpers/fn.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { CapturedArguments } from '@glimmer/interfaces';
import type { Reactive } from '@glimmer/reference';
import { check } from '@glimmer/debug';
import { Formula, isAccessor, unwrapReactive, updateReactive } from '@glimmer/reference';
import { Formula, isMutRef, unwrapReactive, updateReactive } from '@glimmer/reference';
import { buildUntouchableThis, stringifyDebugLabel } from '@glimmer/util';

import { reifyPositional } from '../vm/arguments';
Expand Down Expand Up @@ -82,7 +82,7 @@ export const fn = internalHelper(({ positional }: CapturedArguments) => {

if (import.meta.env.DEV) assertCallbackIsFn(callbackRef);

if (isAccessor(callbackRef)) {
if (isMutRef(callbackRef)) {
let value = args.length > 0 ? args[0] : invocationArgs[0];
return updateReactive(callbackRef, value);
} else {
Expand All @@ -94,7 +94,7 @@ export const fn = internalHelper(({ positional }: CapturedArguments) => {

function assertCallbackIsFn(callbackRef: Reactive | undefined): asserts callbackRef is Reactive {
if (
!(callbackRef && (isAccessor(callbackRef) || typeof unwrapReactive(callbackRef) === 'function'))
!(callbackRef && (isMutRef(callbackRef) || typeof unwrapReactive(callbackRef) === 'function'))
) {
throw new Error(
`You must pass a function as the \`fn\` helper's first argument, you passed ${
Expand Down

0 comments on commit 53031fc

Please sign in to comment.