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

Cannot deploy a perseus app using beta.19 #259

Closed
Miroito opened this issue Feb 23, 2023 · 2 comments · Fixed by #260
Closed

Cannot deploy a perseus app using beta.19 #259

Miroito opened this issue Feb 23, 2023 · 2 comments · Fixed by #260
Labels
A-deployment Area: deployment author-willing-to-impl The author of this issue is willing to try to implement the solution themselves. C-bug Category: bug tribble-reported This issue was reported through Tribble.

Comments

@Miroito
Copy link
Contributor

Miroito commented Feb 23, 2023

This issue is reporting a bug in the code of Perseus. Details of the scope will be available in issue labels.
The author described their issue as follows:

Cannot deploy an app in the beta.19 due to missing item HSR_IGNORE on generic type S in functions of package perseus:

  • get_frozen_global_state_and_register
  • get_frozen_state_and_register

The steps to reproduce this issue are as follows:

I'm not sure whether my app has something special that makes the deployment fail. It might just fail trying to deploy any app.

A minimum reproducible example is available at <>.

  • Hydration-related: false
  • The author is willing to attempt a fix: true
Tribble internal data

dHJpYmJsZS1yZXBvcnRlZCxDLWJ1ZyxBLWRlcGxveW1lbnQsYXV0aG9yLXdpbGxpbmctdG8taW1wbA==

@github-actions github-actions bot added A-deployment Area: deployment author-willing-to-impl The author of this issue is willing to try to implement the solution themselves. C-bug Category: bug tribble-reported This issue was reported through Tribble. labels Feb 23, 2023
@Miroito
Copy link
Contributor Author

Miroito commented Feb 23, 2023

I'm willing to open a PR for this, I want to make sure I have the right idea because I didn't spend enough time in the project to understand what this code does. Here's my idea:

The trait MakeRx looks like this (without the doc comments):

pub trait MakeRx {
    type Rx: MakeUnrx;
    #[cfg(debug_assertions)]
    const HSR_IGNORE: bool = false;
    fn make_rx(self) -> Self::Rx;
}

Meaning HSR_IGNORE is available on types that implement the MakeRx trait only on debug builds.

But the functions get_frozen_state_and_register and get_frozen_global_state_and_register both have this condition:

if *is_hsr && S::HSR_IGNORE {
    return Ok(None);
}

Which has no guard to not be compiled in non-debug builds.

I would propose to simply guard either the full condition, or split it in order to prevent the compiler from tying to find HSR_IGNORE on non-debug builds.
Or perhaps HSR_IGNORE should be defined on all builds and in this case we would just remove it from the trait definition.

Please let me know, I'm willing to open a PR if it is a simple change (again because of my light understanding of what is actually going on there) or feel free to fix it quickly.

PS; I'm surprised something like this would not be picked up while trying to build the library in release mode, I haven't tried though.

@Miroito Miroito changed the title Cannot deploy perseus using beta.19 Cannot deploy a perseus app using beta.19 Feb 23, 2023
@arctic-hen7
Copy link
Member

Oh I'm an idiot, thank you very much, you're absolutely right. PR would be great! (I'll also check up on the CLI tests, which should have caught this...)

arctic-hen7 added a commit that referenced this issue Feb 26, 2023
Previously, they were actually testing the most recently deployed
version because of the way `perseus init` works! This led to issues like #259.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-deployment Area: deployment author-willing-to-impl The author of this issue is willing to try to implement the solution themselves. C-bug Category: bug tribble-reported This issue was reported through Tribble.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants