glimmerjs / glimmer-application

[MOVED] This package is now part of the Glimmer.js monorepo
https://github.com/glimmerjs/glimmer.js
30 stars 13 forks source link

WIP: `renderComponent` args #81

Closed pittst3r closed 7 years ago

pittst3r commented 7 years ago

New signature:

renderComponent(
  component: string,
  parent: Simple.Node,
  options: RenderComponentOptions = {}
): RenderComponentResult

Given:

// From @glimmer/runtime
interface PreparedArguments {
  positional: Array<VersionedPathReference<Opaque>>;
  named: Dict<VersionedPathReference<Opaque>>;
}

type Args = PreparedArguments;

interface RenderComponentOptions {
  nextSibling?: Option<Simple.Node>;
  args?: Args;
}

interface RenderComponentResult {
  update(): void;
}

Also ships with some helpful stuff to make this API easier to use:

import { EMPTY_ARRAY } from '@glimmer/util';
import { prepareNamedArgs } from '@glimmer/application';

// ...

let positional = EMPTY_ARRAY;
let named = prepareNamedArgs({ greeting: 'Hello' });
let result = app.renderComponent('greet-person', containerElement, {
  args: { positional, named }
});

named.greeting.set('Olá');
result.update();

To-do:

chancancode commented 7 years ago

I don't think we can (easily, anyway) support this interface in the VM (or even in userland, for that matter).

This would essentially require support for "dynamic splat" as the updated args can contain keys that are not found during the initial invocation, which would have been impossible with today's static invocation (or even with curried components).

I think it makes sense to move this functionality into the VM, so it's good to align this with the underlying primitives as much as possible. What passing references in the render options? This is how prepareArgs work, so it should be trivial to make it work.

pittst3r commented 7 years ago

@chancancode Okay, so you're saying that we can't easily support an args object that changes shape, but we can support one that keeps its shape?

And yes, I can update it so it accepts args as a reference instead of a POJO.

Any other changes you think I need to make so we can ship this?

chancancode commented 7 years ago

Confirm.

By passing references, it is guaranteed that it won't change shape, because you will just be updating the references directly instead of passing a new dictionary with a possibly different set of keys. You could also take advantage of the existing VM optimizations by passing const references for things you know won't change.

It would be best if you could reuse the interface here (you can duplicate it for now): https://github.com/glimmerjs/glimmer-vm/blob/master/packages/@glimmer/runtime/lib/component/interfaces.ts#L15-L18.

It's basically what you have, except it also supports positional args.

pittst3r commented 7 years ago

@chancancode I updated this per your feedback, seems much better. Also updated the accompanying @glimmer/component PR.

lifeart commented 6 years ago

@robbiepitts why this is closed?

pittst3r commented 6 years ago

@lifeart The API I had was going in an undesirable direction. I think there is something better coming down the pike.

josemarluedke commented 6 years ago

The work for this has began in the Glimmer-vm at https://github.com/glimmerjs/glimmer-vm/pull/701. Not sure if that is already possible to use from a Glimmer.js application.