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

Composing widgets (which begin with liftEffect) #10

Closed
jonathanlking opened this issue Jan 6, 2019 · 7 comments
Closed

Composing widgets (which begin with liftEffect) #10

jonathanlking opened this issue Jan 6, 2019 · 7 comments

Comments

@jonathanlking
Copy link

If I run example 1 <> example 2 we get "example1" and "example2" logged to the console, however only the "example2" button is rendered to the DOM.

example :: forall a. Int -> Widget HTML a
example x = do
  liftEffect (log $ "example" <> show x)
  _ <- button [onClick] [text $ "example" <> show x]
  example’ x

This is not what I expect - I think both buttons should still be rendered. Could you confirm what the correct behaviour should be?

Interestingly moving the logging to after the button click results in both buttons being rendered. I've made a runnable gist that demonstrates this.

@ajnsit
Copy link
Member

ajnsit commented Jan 6, 2019

Yeah the current behaviour is buggy. I'm on it.

ajnsit added a commit that referenced this issue Jan 6, 2019
@ajnsit
Copy link
Member

ajnsit commented Jan 6, 2019

Okay @jonathanlking, I think this should be fixed now. It was happening because purescript-aff tries to be helpful and resolves aff actions synchronously whenever possible. I now detect that condition and handle it correctly.

Please let me know if it works for you now. Please feel free to reopen if it doesn't work as expected.

@ajnsit ajnsit closed this as completed Jan 6, 2019
@jonathanlking
Copy link
Author

Hmm, so running the example from the gist I get both logging, but now neither rendering and a console error: FAILED! TypeError: Cannot read property 'run' of undefined

Did you get this example working? Also thanks for responding and working on a fix so quickly, I really appreciate it!

@ajnsit
Copy link
Member

ajnsit commented Jan 7, 2019

Weird, I thought I tested it and it was working, but I may have had other code changes which got lost. Reopening this.

@ajnsit ajnsit reopened this Jan 7, 2019
ajnsit added a commit that referenced this issue Jan 7, 2019
@ajnsit
Copy link
Member

ajnsit commented Jan 7, 2019

@jonathanlking Can you test out the fix now? I think I now handle the synchronous cases correctly. I also tested the code from your snippet and it seems to work.

@jonathanlking
Copy link
Author

Yes, it looks good to me now too! Thank you so much for this incredible library and amazing support! 🎉

@ajnsit
Copy link
Member

ajnsit commented Jan 7, 2019

Glad to hear it! Thanks for reporting the issue!

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

No branches or pull requests

2 participants