Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Seed's UpdateEl::update method clashes with Itertools #370

Closed
rebo opened this issue Feb 25, 2020 · 4 comments · Fixed by #373
Closed

Seed's UpdateEl::update method clashes with Itertools #370

rebo opened this issue Feb 25, 2020 · 4 comments · Fixed by #373
Assignees

Comments

@rebo
Copy link
Collaborator

rebo commented Feb 25, 2020

This issue was just reported on Discord #support by user BestRanar . When using itertools and iterators in seed views that have not been terminated with a collect::<Vec<_>>() there is a disambiguation compiler error generated from the seed macro.

error[E0034]: multiple applicable items in scope

note: candidate #1 is defined in an impl of the trait 'seed::virtual_dom::update_el::UpdateEl' for the type 'std::iter::FilterMap<_, _>'

note: this error originates in a macro outside of the current crate

Basically rust does not know whether to use Seed's update function or Itertools's update.

Probably the best bet to fix is to change the name of Seed's view macro update() method to something like seed_update(). The reason for this is that Seed's update is internal to the seed macros and in theory not a public api and Itertools is common enough that this clash will come up more frequently.

Another fix is to disambiguate directly in the seed macro, however this prevents implementation of custom UpdateEls by a user if needed.

The temporary fix to the above is to terminate iterators with a collect() if using itertools.

@MartinKavik
Copy link
Member

  • I have the exact problem (itertools clash) in one of my projects.
  • I would rename update method to update_el so it corresponds with naming conventions and similar errors are easier to identify.
  • I would call it with fully quialified syntax to prevent this issue.
  • I'll fix it.

Do you see any problems with the suggested solution?

@MartinKavik MartinKavik self-assigned this Feb 25, 2020
@rebo
Copy link
Collaborator Author

rebo commented Feb 25, 2020

update_el is good.

For fully qualified name syntax would perm fix but we might want to hold off for a bit.

This is because I am getting some milage out of self implementing update to allow dx improvements.

For instance the bind function you have seen works because I have implemented update locally on the return type of bind ( it just delegates update to an EventHandler and a Attr). Other Dx improvements could be made with further experimentation which I couldn't do if the method was called via fully qualified syntax.

I'm thinking things like functions that set both style and inner node blocks (by returning a (Style, Node) typle) for a potential SeedUI.

@MartinKavik
Copy link
Member

Ok, update_el should resolve it enough.

@MartinKavik
Copy link
Member

(can be implemented once #365 is resolved to prevent merge conflicts)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants