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

passing string into is:inline style tag doesn't render said string as child text #11167

Open
1 task done
matthewp opened this issue May 30, 2024 · 6 comments
Open
1 task done
Labels
needs discussion Issue needs to be discussed

Comments

@matthewp
Copy link
Contributor

Astro Info

❯ npx astro info
Astro                    v4.9.2
Node                     v18.20.3
System                   Linux (x64)
Package Manager          npm
Output                   static
Adapter                  none
Integrations             none

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

<style is:inline>{` h1 { color: ${color}; } `}></style>

Renders the code instead.

What's the expected result?

I was sort of expecting it to put the string child in there. Maybe there's a security consideration and set:html is better?

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-ncpwkk?file=src%2Fpages%2Findex.astro

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label May 30, 2024
@bluwy
Copy link
Member

bluwy commented May 31, 2024

I think it's more that the compiler always expect CSS syntax in the <style> tag, so it sees the children as CSS and doesn't do JSX style interpolation. set:html seems like the better way for me.

@matthewp
Copy link
Contributor Author

set:html does work, but I would expect that passing a string as a child to be escaped just like passing a string to any child tag. So I do think this is a case of the compiler having an issue specifically with style tags.

@matthewp
Copy link
Contributor Author

matthewp commented May 31, 2024

(context is that we were talking about removing define:vars possibly in 5.0 and this seems like a fine alternative, hopefully an idea you would like @bluwy 😀)

@bluwy
Copy link
Member

bluwy commented May 31, 2024

Sorry misclosed whle switching tab 😅 Typing my proper comment next

@bluwy bluwy closed this as completed May 31, 2024
@bluwy bluwy reopened this May 31, 2024
@bluwy
Copy link
Member

bluwy commented May 31, 2024

If we have the compiler allowing {} within style tags, and also CSS syntax, wouldn't it be ambiguous when parsing? Unless we hardcode checking {` as the first character.

I'm open to re-thinking how interpolating vars work though. I think the ideal way is something like this?

<style is:inline>
  h1 {
    color: ${color};
  }
</style>

So you can use normal CSS syntax, and easily interpolate variables. I wrote an RFC for Svelte before (which didn't go through), but the conclusion was the same v-bind syntax as Vue as it's valid CSS syntax. We also get better syntax highlighting this way.

@matthewp
Copy link
Contributor Author

matthewp commented Jun 7, 2024

@bluwy I do like that idea as a feature.

I don't think this would be ambiguous though. { is not a valid first character of a stylesheet is it? If so then I would agree with you.

@matthewp matthewp added needs discussion Issue needs to be discussed and removed needs triage Issue needs to be triaged labels Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs discussion Issue needs to be discussed
Projects
None yet
Development

No branches or pull requests

2 participants