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 compile error when using comment and doctype tags within the view! macro #2355

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

thinety
Copy link

@thinety thinety commented Feb 22, 2024

Discussed in #2338.

Preview of the error messages:
image

The first commit contains just style changes (using proc_macro_error::abort! everywhere instead of just abort!, to be consistent with proc_macro_error::emit_error!), so it can be reverted if desired.

Just for consistency. `proc_macro_error::emit_error` is always called in
the qualified form, but `proc_macro_error::abort!` was sometimes called
in the non-qualified form, which was made possible by
`#[macro_use] external crate proc_macro_error`.
Currently, these tags are not displayed in the rendered view. Instead
of silently doing so, this change introduces a compile error with a
helpful message.
@gbj gbj added this to the 0.7 milestone Feb 27, 2024
@gbj
Copy link
Collaborator

gbj commented Feb 27, 2024

Thanks. This is somewhat awkward as it's a breaking change. I think the <!DOCTYPE> change is unlikely to affect many use cases, but the comment one may be more widely used -- and turning something from a noop to a compile error is definitely a problem for people's CI/CD and so on, if we release it.

Maybe just change it for DOCTYPE, or we can just wait?

Emit compile-time warnings instead of errors when using doctype and
comment tags within `view!` macro.
@thinety
Copy link
Author

thinety commented Feb 27, 2024

I've changed the compile-time errors to warnings:
image
This only works in nightly though.

Up to you if you want to leave this change alone and just wait for the next major.

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

Successfully merging this pull request may close these issues.

None yet

2 participants