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

Optional Attributes Chapter 2 #1550

Closed
siku2 opened this issue Sep 10, 2020 · 3 comments · Fixed by #1637
Closed

Optional Attributes Chapter 2 #1550

siku2 opened this issue Sep 10, 2020 · 3 comments · Fixed by #1637
Milestone

Comments

@siku2
Copy link
Member

siku2 commented Sep 10, 2020

Optional attributes were implemented in #1537, but they require the use of a special syntax ?=. This was somewhat required for the first implementation because of the internal handling of attributes but the very same PR also changed that so it's no longer a requirement.

Initial discussion: #1537 (comment).

Required changes

  1. Tag attributes which support the optional attribute syntax should instead accept values that implement Into<Option<_>>.
  2. Optional component properties (i.e. props marked with #[prop_or*], not necessarily with value Option<_>) should also accept Into<Option<_>>. If the variant is None, the default value should remain. Hopefully this doesn't cause any type inference problems for the Transformer...
  3. The documentation needs to be updated in a few places.
@siku2 siku2 added this to the v0.18 milestone Sep 10, 2020
@jstarry
Copy link
Member

jstarry commented Sep 12, 2020

@siku2 I agree will all the required changes, thanks for the write up. Also hoping we don't run into type inference problems 🤞

@ranile
Copy link
Member

ranile commented Jul 18, 2021

Should this issue be closed, now that the implementation is done?

@siku2
Copy link
Member Author

siku2 commented Jul 18, 2021

Should this issue be closed, now that the implementation is done?

You're right!

@siku2 siku2 closed this as completed Jul 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants