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

Deployed apps don't reevaluate cells marked as 'Reevaluates automatically' #2568

Closed
ahamez opened this issue Apr 14, 2024 · 6 comments · Fixed by #2569
Closed

Deployed apps don't reevaluate cells marked as 'Reevaluates automatically' #2568

ahamez opened this issue Apr 14, 2024 · 6 comments · Fixed by #2569

Comments

@ahamez
Copy link
Contributor

ahamez commented Apr 14, 2024

Environment

  • Elixir & Erlang/OTP versions (elixir --version): v1.15.7
  • Operating system: macOS 14.4.1
  • How have you started Livebook (mix phx.server, livebook CLI, Docker, etc): macOS app
  • Livebook version (use git rev-parse HEAD if running with mix): v0.12.1
  • Browsers that reproduce this bug (the more the merrier): Safari, Firefox
  • Include what is logged in the browser console: N/A
  • Include what is logged to the server console: N/A

Current behavior

Deployed apps don't reevaluate cells marked as 'Reevaluates automatically'.

For instance, in this example, when selecting a new entry in the dropdown menu in the deployed app, the following cell is not evaluated automatically.

However, it's the case when the entry is changed in Livebook (thus, not in deployed apps).

Expected behavior

I would expect the cells to be reevaluated automatically to offer a consistent experience.

@josevalim
Copy link
Contributor

@jonatanklosko do you remember why we disable reevaluate automatically for apps? I think it makes sense to keep due to the input behaviour, no?

@jonatanklosko
Copy link
Member

jonatanklosko commented Apr 15, 2024

@josevalim it may have been a side effect, we first had automatic reevaluation for everything (#1928), so we skipped the logic for "automatically reevaluating" cells, and then we reverted to reevaluate changes with a button (#2066), but the "automatically reevaluating" logic is still skipped. I don't remember nor couldn't find if we actually discussed "automatically reevaluating" cells specifically.

So with that context, I think it's fine to allow them. It should cover some of the use cases where reevaluating manually is awkward. And it's optional, so it doesn't break the use cases where we want to avoid eager reevaluation. Hopefully it's not confusing to have yet another behaviour in the equation, but in terms of consistency it probably makes sense.

@jonatanklosko
Copy link
Member

Oh, looking at #2066, this test indicates it could've been intentional.

@jonatanklosko
Copy link
Member

@josevalim perhaps initially we said that "automatically reevaluating" doesn't apply to apps and just rolled with it. Anyway, I'm ok making the change, wdyt?

@josevalim
Copy link
Contributor

Agreed. I also think we forgot about the input cases and I would reenable it :)

@ahamez
Copy link
Contributor Author

ahamez commented Apr 15, 2024

Thanks! This small change will make much easier to deploy prototype web apps 🙏

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 a pull request may close this issue.

3 participants