-
Notifications
You must be signed in to change notification settings - Fork 596
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
Asynchronous render #646
base: master
Are you sure you want to change the base?
Asynchronous render #646
Conversation
This code requires that any browser or node instance that doesn't support async be compiled using babel and the polyfill being included. Being that we still have support enabled for IE 11 and Safari (browser which represent more than 1% of usage), this adds a lot of weight to choo. (babel-poylfill is around 85k depending on compression which is MUCH larger than choo) |
I just realised that, I'm removing async/await now |
Also, check out my comments in #645 -- there are other ways to do this without needing to change how choo works |
I forgot to mention but this change is also backward compatible, normal sync rendering will still work |
|
Does it have to be included in core? It could be up to the developer to decide if she wants it or not and which shim to use. Otherwise we could use |
I think this whole PR could fit for a store. Since the the |
@YerkoPalma that's actually a good idea. I could definitely do it but, there is a recent change to delegate store initialisation (#638), it makes it harder since the patching would only be applied after calling |
@toddself I updated the PR so that the a |
I really like this implementation, as it brings the best of both worlds and it's not a breaking change. |
Any update on this? :) Anything a noob contributor (me lol) could help out with? |
Fixes #645