Pass root in second argument to app().

[jorgebucaran]

The proposal is to remove the app's root property and do the same passing it in the second argument to app().

Before

app({  init, view, state, actions, root }) 

After

app({ init, view, state, actions }, root) 

e.g.

app(
  { 
    init, 
    view, 
    state, 
    actions 
  }, 
  document.getElementById("app")
)

Pros

• Make app() props have to deal only with the state management side of things.
• Other libraries often do the same, e.g., P/React's render(elm, root), Elm's Elm.Main.embed(root), mithril's m,mount(document.body, Counter) (the order is reversed here though), Vue's $mount, etc.
• Cuts like 10 bytes. 😅

Cons

• There is nothing wrong with the current way of doing things.
• Adds like 10 bytes.
• Complicates the signature of HOAs. Thanks @okwolf.

[okwolf]

@jorgebucaran how would this work with HOAs? Would they need to accept multiple args as well, and call the app function with both the props and root?

[jorgebucaran]

@okwolf Oh, I didn't think about it. I am starting to dislike this proposal haha.

[pspeter3]

You could possibly separate the app factory and the render factory.

[SkaterDad]

I think the current API works well. It's nice and clean.

…

On Sun, Oct 8, 2017, 11:10 AM Phips Peter ***@***.***> wrote: You could possibly separate the app factory and the render factory. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#410 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AMpDr-4gQUvoiMGdeFHjrFs-ghMXLeBNks5sqPQIgaJpZM4PxvKh> .

[lukejacksonn]

I don't expect many people will actually include a root prop that often anyway, but instead let it fallback to default (document.body). I like the idea but the cons seem to outweigh the pros.

[jorgebucaran]

Thanks everyone! I'll keep things the way they are. 👍

[SkaterDad]

Out of curiosity, what's the benefit of this change? I can't access Slack from work to read the conversation.

[jorgebucaran]

Now, with HOA builtin support pushed to userland, we can revive this and as per discussion on Slack. Shall we implement it?

[pspeter3]

I prefer it but I do think we should have principles for why to answer @SkaterDad

[jorgebucaran]

The real (not really) question is, how do we feel about formatters like prettier making the code this:

app(
  { 
    init, 
    view, 
    state, 
    actions 
  }, 
  document.getElementById("app")
)

[jorgebucaran]

@SkaterDad The convo was basically that since builtin HOA support is out, and:

Complicates the signature of HOAs. Thanks @okwolf.

...was one of the reasons to turn this down, we can implement it now. Preact/React/Inferno users trying Hyperapp will feel at home and we can save some bytes too.

[pspeter3]

I still want to make sure we have consistent reasoning for API changes. As we approach 1.0, the API will become locked.

[jorgebucaran]

Well, let's approach it this way. What's not to like about

app(props, container)

app(
  { 
    init, 
    view, 
    state, 
    actions 
  }, 
  document.getElementById("app")
)

app({ init, view, state, actions }, document.getElementById("app"))

[okwolf]

@jorgebucaran FYI using this also breaks your compose function.

[jorgebucaran]

@okwolf Thanks, I'll update it.