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

Add no_implicit_prelude to proc macro tests #2033

Merged
merged 5 commits into from
Sep 5, 2021

Conversation

mc1098
Copy link
Contributor

@mc1098 mc1098 commented Sep 5, 2021

Description

Adds no_implicit_prelude to the passing tests for all the proc macros and resolved an issue with fully qualified Options in the derive_props macro.
Fixes the function_components macro tests so trybuild actually runs them :)

Fixes #0000
None - but was prompted by this comment and checks some boxes in #1633

Checklist

  • I have run cargo make pr-flow
  • I have reviewed my own code
  • I have added tests (does reenabling them count 😅?)

mc1098 added 5 commits September 5, 2021 18:14
function_component macro tests weren't being run by try build due to
change in dir name. Imports corrected now that function_component is now
in yew.

Adds no_implicit_prelude to *-pass tests
Copy link
Contributor Author

@mc1098 mc1098 left a comment

Choose a reason for hiding this comment

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

I don't think there is anyway to prevent someone from using:

use Result as Option;

to pass is the path an Option check. This is the same as before this change but now at least if someone uses a fully qualified path it will still work, which was failing one of the tests because of no_implicit_prelude requiring me to use a fully qualified path.

Not sure whether we should just enforce users to use the attributes provided for derive_props and not treat Option as a special case?

@siku2
Copy link
Member

siku2 commented Sep 5, 2021

Not sure whether we should just enforce users to use the attributes provided for derive_props and not treat Option as a special case?

I think it's reasonable to treat Option specially because of how naturally optional props fit into the whole HTML environment. Of course, the way we're detecting the Option type isn't perfect, but I think it's robust enough for all reasonable use cases.

Btw, really appreciate this work! This is something I've been meaning to do for a while but never got around to.

@siku2 siku2 merged commit 58753d9 into yewstack:master Sep 5, 2021
@mc1098 mc1098 deleted the proc-mac-hygiene branch September 5, 2021 22:50
@Xavientois
Copy link
Contributor

Nice to see better macro hygiene. Just wondering why we added the verbose syntax to our own tests?

@voidpumpkin voidpumpkin added the A-yew-macro Area: The yew-macro crate label Nov 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-yew-macro Area: The yew-macro crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants