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

Skip frontend packages install if previously done #2400

Merged
merged 4 commits into from
Jan 17, 2024
Merged

Conversation

jackie-pc
Copy link
Contributor

@jackie-pc jackie-pc commented Jan 16, 2024

We introduce a decorator @cached_procedure for this functionality. "Procedure" because we are not caching any results, only the inputs to a previous call. It is up to the decoratee to ensure its args capture the unique call under cache sufficiently.

@jackie-pc jackie-pc marked this pull request as ready for review January 16, 2024 19:57
@jackie-pc jackie-pc requested review from picklelo, masenf and Lendemor and removed request for picklelo January 16, 2024 19:57
Copy link
Collaborator

@masenf masenf left a comment

Choose a reason for hiding this comment

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

as i'm reading it, this will also result in frontend packages not being installed for export and run --env prod if the packages have already been installed.

seems the only way to get package installation to occur now is to change something in the app the requires a different set of packages, or re-init the project.

@jackie-pc
Copy link
Contributor Author

jackie-pc commented Jan 17, 2024

Is it necessary or desirable to perform real frontend install steps under those conditions? It's really a case of capturing all such requirements in the cache payload (e.g. by including them as args to the function and rolling it into the payload_fn).

@masenf
Copy link
Collaborator

masenf commented Jan 17, 2024

Is it necessary or desirable to perform real frontend install steps under those conditions?

i dont think it's necessary, just a potentially notable change in how those commands work.

i mark approve, i think we should take it; but also note in the docs that if you want to actually reinstall packages to use reflex init first.

@picklelo picklelo merged commit 2c27058 into main Jan 17, 2024
@picklelo picklelo deleted the jackie-cached-install branch January 30, 2024 03:09
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.

3 participants