Skip to content
This repository was archived by the owner on Feb 26, 2022. It is now read-only.

Bug 940510 - Initial draft of signals JEP.#1316

Merged
Gozala merged 3 commits into
mozilla:JEPfrom
Gozala:jeps/signals
Apr 6, 2015
Merged

Bug 940510 - Initial draft of signals JEP.#1316
Gozala merged 3 commits into
mozilla:JEPfrom
Gozala:jeps/signals

Conversation

@Gozala

@Gozala Gozala commented Dec 3, 2013

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread Signal.md

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I understand lift, but I don't understand this example

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nm, I think I do -- does this also update the value of Y when X is updated, or is it evaluated lazily?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I understand lift, but I don't understand this example
nm, I think I do -- does this also update the value of Y when X is updated, or is it evaluated lazily?

Yes Y is updated every time X is updated, but also yes on laziness. Neither Y or X will get updated unless they are consumed, in other words there should be write(..., Y) somewhere or write(..., Z) where Z is derived from Y.

Note: Above being said Z also could be derived from X instead, in such case Y won't be updated unless Y or some other W (which is it's derived signal) is written.

I go into details of this later in the document, but if it's still not clear, we should find a way to make it so.

@jsantell

jsantell commented Dec 3, 2013

Copy link
Copy Markdown
Contributor

I don't quite understand implementation-wise, how to do things like creating a signal, ending a signal, reading/unwrapping from a signal

Comment thread Signal.md Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would this not be // XY x: --x1-y1---x2--y2----x3-y3-- based on what was said before "biased towards the first signal if both signals update at the same time" ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the bias is that the x3 value is the last/current value.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would this not be // XY x: --x1-y1---x2--y2----x3-y3-- based on what was said before "biased towards the first signal if both signals
update at the same time" ?

Yes you right it should be x3 y3

@erikvold

erikvold commented Dec 5, 2013

Copy link
Copy Markdown
Contributor

Hey @Gozala it looks really good! I'd like to see some examples on how to end a signal though. Also I'm not sure I like the write/writer, is there not some way that updateMenuItems could be called directly and avoid the write/writer step. Like write(updateMenuItems, signal) ? and rename write to process (which you suggested in irc) ?

@jsantell

jsantell commented Dec 5, 2013

Copy link
Copy Markdown
Contributor

Agree with Erik -- looks good, but this is just from a consuming perspective -- it's not clear how to create, modify or end signals if we were to create a signal-based module

@Gozala

Gozala commented Dec 6, 2013

Copy link
Copy Markdown
Contributor Author

So I talked over with Evan and got blessing from him too, we also discussed write API and he suggested something like:

const reactor = new Reactor({
  onStart: () => {},
  onEnd: () => {},
  onNext: () => {},
})
reactor.run(input);

thoughts ?

@Gozala

Gozala commented Dec 6, 2013

Copy link
Copy Markdown
Contributor Author

Another alternative could be something something more high level:

Assuming that MenuItems is a signal that starts & ends with empty hash and
on updates is like:

{ "sdk-menu-item-1": { id: "sdk-menu-item-1", label: "foo", owners: ["window-id-1", "window-id-2"],
  "sdk-menu-item-2": { id: "sdk-menu-item-2", label: "foo", owners: ["window-id-1", "window-id-2"],
  ....
 }

One should be able to define MenuItem class as follows:

const MenuItem = Model({
   // Called any time new value added to a hash
   setup: () => {},
   // Called each time value disappears form hash
   dispose: () => {},
   // Called each value in hash changes
   update: () => {},
   // ....
}, MenuItems);

@Gozala

Gozala commented Dec 6, 2013

Copy link
Copy Markdown
Contributor Author

I guess owners could be actual set of windows instead of window IDs.

@Gozala

Gozala commented Dec 6, 2013

Copy link
Copy Markdown
Contributor Author

I think @jsantell suggested something like this instead:

const MenuItem = Class({
   extends: Reactor,
   // input signal (I hate name `react`)
   react: MenuItems,
   // Called any time new value added to a hash
   setup: () => {},
   // Called each time value disappears form hash
   dispose: () => {},
   // Called each value in hash changes
   update: () => {},
   // ....
});

@Gozala

Gozala commented Dec 6, 2013

Copy link
Copy Markdown
Contributor Author

I guess I would prefer something like:

const MenuItem = Class({
   // Called any time new value added to a hash
   setup: () => {},
   // Called each time value disappears form hash
   dispose: () => {},
   // Called each value in hash changes
   update: () => {},
   // ....
});

Reactor.run(MenuItem, input);

@Gozala

Gozala commented Dec 6, 2013

Copy link
Copy Markdown
Contributor Author

@jsantell @erikvold I have updated some parts to reflect received feedback. To be specific I rewrote section about laziness and changed most of it to be oriented around how to define a signal. I have also rewrote part about consuming signals in order to replace write that no one liked with Reactor({ ... }) that @jsantell agreed was more
clear.

Gozala added a commit that referenced this pull request Apr 6, 2015
Bug 940510 - Initial draft of signals JEP.
@Gozala
Gozala merged commit fd670d6 into mozilla:JEP Apr 6, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants