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

@myself can work by accident #895

Closed
benwilson512 opened this issue May 16, 2020 · 5 comments
Closed

@myself can work by accident #895

benwilson512 opened this issue May 16, 2020 · 5 comments

Comments

@benwilson512
Copy link

benwilson512 commented May 16, 2020

Hey folks,

Recently when attempting to use send_update on 0.12.1 I created something that worked by accident, and then later broke, which was very confusing.

The short version is that I was doing this inside a live component:

Task.async(fn ->
result = HTTP.get(url)

{:result, socket.assigns.myself, result}
end)

And then in the parent doing:

def handle_info({_, {:result, component_id, result}, socket) do
  send_update(FormComponent, id: component_id, result: result)
  {:noreply, socket}
end

This shouldn't work. @myself is documented to be the "internal unique reference" to the component. However, it did work because at the end of the day @myself is merely an integer counting up from 0. There were 2 components on the page, so it had a value of 1. Also, I was editing the form component for product id 1, so voila, the value worked.

I should have been doing {:result, socket.assigns.id, result}.

I propose that @myself should be namespaced in some way to make it very difficult for it to accidentally collide with a specific external component id.

@benwilson512
Copy link
Author

Tangentially, a follow on proposal:

send_update should raise if the component doesn't exist

Currently the behaviour is that nothing happens. This makes it difficult to sort out where the problem is. Is the problem that I'm not using the right component id? Or is the issue that I'm not doing the right thing in update/2? It's quite hard to say.

@josevalim
Copy link
Member

For the first issue, maybe we can make @myself return %Phoenix.LiveComponent.CID{cid: int}. Then we implement Phoenix.HTML.Safe for said struct that simply returns the cid as a string.

For the second issue, there is #739. Note we should warn and not raise, as race conditions could make it so the component no longer is on the page. For example, imagine you do an async request that should "reply" to the component, but someone closes the component on the UI.

@benwilson512
Copy link
Author

benwilson512 commented May 16, 2020

Good point about #739. A warning or even a debug level thing would all work fine for the purpose of debugging in development, so cool!

With respect to the main issue, if I see a CID (presumably Component ID) I guess I still don't have a strong sense that this is something internal vs something that the parent has. Can you elaborate on why @myself and @id could ever not be the same? That'd be another route here too.

@josevalim
Copy link
Member

@benwilson512 the ID is something you control and it can be anything. The CID is completely internal to LV and is used to communicate between client/server.

@benwilson512
Copy link
Author

OK great, makes sense.

henrik added a commit to henrik/phoenix_live_view that referenced this issue Jun 6, 2020
…able

Example of the race condition mentioned, from phoenixframework#895 (comment):

> Note we should warn and not raise, as race conditions could make it so the component no longer is on the page.
> For example, imagine you do an async request that should "reply" to the component, but someone closes the component on the UI.

Closes phoenixframework#739. Refs phoenixframework#957.
henrik added a commit to henrik/phoenix_live_view that referenced this issue Jun 6, 2020
…able

Example of the race condition mentioned, from phoenixframework#895 (comment):

> Note we should warn and not raise, as race conditions could make it so the component no longer is on the page.
> For example, imagine you do an async request that should "reply" to the component, but someone closes the component on the UI.

Closes phoenixframework#739. Refs phoenixframework#957.
henrik added a commit to henrik/phoenix_live_view that referenced this issue Jun 7, 2020
…able

Example of the race condition mentioned, from phoenixframework#895 (comment):

> Note we should warn and not raise, as race conditions could make it so the component no longer is on the page.
> For example, imagine you do an async request that should "reply" to the component, but someone closes the component on the UI.

Closes phoenixframework#739. Refs phoenixframework#957.
josevalim pushed a commit that referenced this issue Jun 7, 2020
…able

Example of the race condition mentioned, from #895 (comment):

> Note we should warn and not raise, as race conditions could make it so the component no longer is on the page.
> For example, imagine you do an async request that should "reply" to the component, but someone closes the component on the UI.

Closes #739. Refs #957.
henrik added a commit to henrik/phoenix_live_view that referenced this issue Jun 8, 2020
Harder to get it mixed up with things like `assigns.id`.

Closes phoenixframework#895.
henrik added a commit to henrik/phoenix_live_view that referenced this issue Jun 8, 2020
Harder to get it mixed up with things like `assigns.id`.

Closes phoenixframework#895.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants