Skip to content
This repository has been archived by the owner on Jul 19, 2020. It is now read-only.

Stdweb spawn local future #38

Merged
merged 6 commits into from
Dec 11, 2019

Conversation

izissise
Copy link
Contributor

@izissise izissise commented Dec 10, 2019

  • Fix not equal assign example
  • Rename send_self send_self_batch to send_message send_message_batch
  • Use stdweb::spawn_local to execute futures sent to send_future
  • Add AgentLinkFuture trait and implementation

Futures successfully executed and sent back message to the Component/Agent using cargo web and asmjs target.
Also tested with wasm32-unknown-unknown target and it work great

Issue: yewstack/yew#776

It depends on yewstack/yew#802 for AgentLinkFuture to work

@hgzimmerman hgzimmerman self-requested a review December 10, 2019 23:31
Copy link
Member

@hgzimmerman hgzimmerman left a comment

Choose a reason for hiding this comment

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

Thanks for taking the time to do this!

My only reservation I have about this is discarding web-sys implementation entirely.
There are future plans in Yew to support parallel implementations that rely on either web-sys or stdweb, so the old web-sys based futures support may come in handy later.

Since I can't come up with a scheme that makes feature-gating either stdweb or web-sys support practical right now, I'll resurrect the old implementation myself when it comes time to do so.

Just remove the unneeded dependencies from the cargo.toml and I think that this is good to merge when Yew merges the associated PR there.

Cargo.toml Show resolved Hide resolved
src/future.rs Outdated Show resolved Hide resolved
@hgzimmerman hgzimmerman merged commit 3e5a2a2 into yewstack:master Dec 11, 2019
@hgzimmerman
Copy link
Member

Update, apparently this feature required that experimental_features_which_may_break_on_minor_version_bumps be a feature on stdweb. I'll open another PR to add that.

@izissise
Copy link
Contributor Author

A damn, I wasn't sure it was needed, sorry about that

@hgzimmerman
Copy link
Member

No problem

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

Successfully merging this pull request may close these issues.

2 participants