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 life-cycle diagram to contain preload/1 on change #2515

Merged
merged 1 commit into from
Mar 10, 2023
Merged

Fix life-cycle diagram to contain preload/1 on change #2515

merged 1 commit into from
Mar 10, 2023

Conversation

aiwaiwa
Copy link
Contributor

@aiwaiwa aiwaiwa commented Mar 10, 2023

As these paragraphs point out

(quote)

So on first render, the following callbacks will be invoked:

preload(list_of_assigns) -> mount(socket) -> update(assigns, socket) -> render(assigns)

On subsequent renders, these callbacks will be invoked:

preload(list_of_assigns) -> update(assigns, socket) -> render(assigns)

(end quote)

the diagram needs to reflect those arrows.

Additionally, I added a transparent styling to a pure layout types of sub-graphs.

image
image

@josevalim
Copy link
Member

Hi @aiwaiwa! I think update/2 should point to "any changes?" as well.

Also, maybe instead of "ever mounted?", could we perhaps keep only "mount/1" and say "mount/1
only once".

Thoughts?

@aiwaiwa
Copy link
Contributor Author

aiwaiwa commented Mar 10, 2023

Something like this (it's a draft, ignore the styling)?

image

That really simplified it! Do you think it might be confusing that initial render goes through "any changes?"

@josevalim
Copy link
Member

That really simplified it! Do you think it might be confusing that initial render goes through "any changes?"

Not at all. The initial render, after all, always has changes :D

@josevalim josevalim merged commit b54359e into phoenixframework:main Mar 10, 2023
@josevalim
Copy link
Member

💚 💙 💜 💛 ❤️

@aiwaiwa aiwaiwa deleted the improve-live-component-diagram branch March 10, 2023 21:30
@aiwaiwa
Copy link
Contributor Author

aiwaiwa commented Mar 10, 2023

I'll re-PR, I was only asking if that was what you wanted

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

Successfully merging this pull request may close these issues.

2 participants