Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix strictness #16

Merged
merged 1 commit into from
Jun 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/core/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,15 @@ npm install @remote-ui/core --save
This function accepts two arguments:

- `channel` is a `RemoteChannel`, a function that will be called with serialized representation of UI updates. [`RemoteReceiver#receive`](#remotereceiverreceive) is one such function, and most uses of remote-ui will rely on connecting those two “sides” by passing the `receive` method for this argument.
- `options` is an optional options object. There is currently one supported option: `components`. This value is the list of components that can be constructed and attached to this root. This is necessary because, by default, this library does not supply any components to render; you are responsible for implementing a component API that makes sense for your use case.
- `options` is an optional options object. There is currently one supported option: `strict`, which is enabled by default. When enabled, all `props` and `children` for remote components will be frozen (with `Object.freeze()`) in order to prevent direct mutation of those values (to prevent unexpected behavior, all mutations to these values should be done with the [`RemoteComponent`](#remotecomponent) API). The default `strict`ness also prevents potentially-untrusted code from adding properties or children that it is not supposed to. However, `Object.freeze` does have a small runtime cost, so if you are comfortable without this safety, you can disable it by passing `strict: false`:

```ts
import {createRemoteRoot, RemoteReceiver} from '@remote-ui/core';

const receiver = new RemoteReceiver();

const root = createRemoteRoot(receiver.receive, {
components: ['Button', 'TextField', 'Card'],
strict: false,
});
```

Expand Down
33 changes: 22 additions & 11 deletions packages/core/src/root.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ import type {
export interface Options<
AllowedComponents extends RemoteComponentType<string, any>
> {
readonly components: readonly AllowedComponents[];
readonly strict?: boolean;
readonly components?: readonly AllowedComponents[];
}

export function createRemoteRoot<
Expand All @@ -30,7 +31,7 @@ export function createRemoteRoot<
AllowedChildrenTypes extends AllowedComponents | boolean = true
>(
channel: RemoteChannel,
_options?: Options<AllowedComponents>,
{strict = true}: Options<AllowedComponents> = {},
): RemoteRoot<AllowedComponents, AllowedChildrenTypes> {
type Root = RemoteRoot<AllowedComponents, AllowedChildrenTypes>;
type Component = RemoteComponent<AllowedComponents, Root>;
Expand Down Expand Up @@ -102,8 +103,8 @@ export function createRemoteRoot<

makePartOfTree(component);
makeRemote(component, id, remoteRoot);
props.set(component, initialProps);
children.set(component, []);
props.set(component, strict ? Object.freeze(initialProps) : initialProps);
children.set(component, strict ? Object.freeze([]) : []);

return (component as unknown) as RemoteComponent<typeof type, Root>;
},
Expand Down Expand Up @@ -207,10 +208,9 @@ export function createRemoteRoot<
return perform(component, {
remote: (channel) => channel(ACTION_UPDATE_PROPS, component.id, newProps),
local: () => {
props.set(
component,
Object.freeze({...props.get(component), ...newProps}),
);
const mergedProps = {...props.get(component), ...newProps};

props.set(component, strict ? Object.freeze(mergedProps) : mergedProps);
},
});
}
Expand All @@ -237,9 +237,14 @@ export function createRemoteRoot<
tops.set(descendant, newTop),
);

const mergedChildren = [
...(children.get(container) ?? []),
normalizedChild,
];

children.set(
container,
Object.freeze([...(children.get(container) ?? []), normalizedChild]),
strict ? Object.freeze(mergedChildren) : mergedChildren,
);
},
});
Expand Down Expand Up @@ -270,7 +275,10 @@ export function createRemoteRoot<

const newChildren = [...(children.get(container) ?? [])];
newChildren.splice(newChildren.indexOf(child as any), 1);
children.set(container, Object.freeze(newChildren));
children.set(
container,
strict ? Object.freeze(newChildren) : newChildren,
);
},
});
}
Expand Down Expand Up @@ -298,7 +306,10 @@ export function createRemoteRoot<

const newChildren = [...(children.get(container) || [])];
newChildren.splice(newChildren.indexOf(before as any), 0, child);
children.set(container, Object.freeze(newChildren));
children.set(
container,
strict ? Object.freeze(newChildren) : newChildren,
);
},
});
}
Expand Down
12 changes: 6 additions & 6 deletions packages/react/src/host/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export function useAttached<T extends Attachable>(
// If the subscription has been updated, we'll schedule another update with React.
// React will process this update immediately, so the old subscription value won't be committed.
// It is still nice to avoid returning a mismatched value though, so let's override the return value.
returnValue = receiver.get(attached);
returnValue = {...receiver.get(attached)};

setState({
receiver,
Expand All @@ -56,8 +56,6 @@ export function useAttached<T extends Attachable>(
return;
}

const value = receiver.get(attached);

setState((previousState) => {
const {
receiver: previousReceiver,
Expand All @@ -69,13 +67,15 @@ export function useAttached<T extends Attachable>(
return previousState;
}

const value = {...receiver.get(attached)};

// If the value hasn't changed, no update is needed.
// Return state as-is so React can bail out and avoid an unnecessary render.
if (deepEqual(previousValue, value)) {
if (shallowEqual(previousValue, value)) {
return previousState;
}

return {...previousState, value: {...value}};
return {receiver, value};
});
};

Expand All @@ -93,7 +93,7 @@ export function useAttached<T extends Attachable>(
return returnValue;
}

function deepEqual<T>(one: T, two: T) {
function shallowEqual<T>(one: T, two: T) {
return Object.keys(two).every(
(key) => (one as any)[key] === (two as any)[key],
);
Expand Down