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

Proposal: instead of not rendering unknown views, fall back to View and continue to render children #1413

Open
bcardarella opened this issue Aug 19, 2024 · 6 comments
Assignees
Milestone

Comments

@bcardarella
Copy link
Collaborator

bcardarella commented Aug 19, 2024

I think our approach to not render unknown views and not rendering children is the wrong. I believe instead we should convert to a View or some other generic view, still apply modifiers, and continue rendering children.

This of course is a post v0.3 thing but let's discuss if this is in fact a better approach.

This would be consistent with the web:

Screenshot 2024-08-19 at 7 27 32 PM
@carson-katri
Copy link
Contributor

I would do this by using Group when the element doesn't exist. Group automatically applies its modifiers to each child.

We should probably still log the error. It could be something like this:

<nonexistent style="foregroundStyle(.orange);">
  <Text>Hello, world!</Text>
</nonexistent>

would render as:

Group {
  Text("Hello, world!")
}
.foregroundStyle(.orange)
.task {
  logger.log(.warning, "...")
}

with a message like this:

Unknown element `<nonexistent>`. The app may not render as expected.

It would be nice if we had some debug information passed on all elements. I think LiveView does this for a jump-to-definition feature. It could use this info to log the file/line number of the unknown element.

@bcardarella
Copy link
Collaborator Author

bcardarella commented Aug 20, 2024 via email

@carson-katri
Copy link
Contributor

carson-katri commented Nov 5, 2024

Currently, this is catching elements such as <Style> and <iframe>, which seems incorrect. Should we just manually exclude these tag names?

'<csrf-token>' is not a known element.
'<Style>' is not a known element.
'<iframe>' is not a known element.
'<nonexistent>' is not a known element.

@bcardarella
Copy link
Collaborator Author

I suspect we'll need to split between HEAD and BODY here to really manage this concern properly. Thoughts?

@carson-katri
Copy link
Contributor

carson-katri commented Nov 5, 2024

Yeah, that makes sense to me. The iframe may still need to be handled separately, since I don't think it'd be placed in the head.

@bcardarella
Copy link
Collaborator Author

I can change the plug to inject into whatever part of the docuemnt we want, it's really only in dev environment so render perf isn't an issue if I were parse the documents and walk into a specific part

@bcardarella bcardarella removed the v0.4 label Feb 27, 2025
@bcardarella bcardarella added this to the 0.5.0 milestone Feb 27, 2025
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

No branches or pull requests

2 participants