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

Updating Template before CSS causes template to not pick up scoped class name #90

Open
jakebixby opened this issue Aug 2, 2023 · 4 comments

Comments

@jakebixby
Copy link
Contributor

jakebixby commented Aug 2, 2023

I've observed some funky behavior within the new addon, specifically around creating elements with a style class before the class exists, the template never receives the scoped class definition, even after the class has been created and the template has been changed. Additional classes don't necessarily trigger a scoped class name update, only in certain cases where the element has some specific changes to it.

  1. Create a template file containing a div with a single style
// hbs
<div class="foo">foo</div>
  1. Create an accompanying css file with the style for the div
// css
.foo {
 	color: red;
 }
  1. Observe the rendered HTML does not have a scoped class name for foo
// html output
<div class="foo">foo</div>
  1. Update the template file with a second div with a second single style
// hbs
<div class="foo">foo</div>
<div class="bar">bar</div>
  1. Observe the rendered HTML after build has applied the scoped class name for foo
// html output
<div class="foo_ee3b6cad0">foo</div>
<div class="bar">bar</div>
  1. Update the CSS file with a style definition for bar
// css
.foo {
 	color: red;
 }
 .bar {
 	color: green;
 }
  1. Observe the rendered HTML after build to see there has been no change to the scoped class definitions.
// html output
<div class="foo_ee3b6cad0">foo</div>
<div class="bar">bar</div>
  1. Save the template file with no changes
// hbs
<div class="foo">foo</div>
<div class="bar">bar</div>
  1. Observe the rendered HTML after build to see there has been no change to the scoped class definitions.
// html output
<div class="foo_ee3b6cad0">foo</div>
<div class="bar">bar</div>
  1. Add a single space character after "bar" and save the template file
// hbs
<div class="foo">foo</div>
<div class="bar">bar </div>
  1. Observe the rendered HTML after build to see the first definition is scoped, but there has been no change to the second scoped class definition.
// html output
<div class="foo_ee3b6cad0">foo</div>
<div class="bar">bar </div>
  1. Add a div with no styles after the "bar" div and save the template file
// hbs
<div class="foo">foo</div>
<div class="bar">bar </div>
<div>baz</div>
  1. Observe the rendered HTML after build to see there has been no change to the scoped class definitions.
// html output
<div class="foo_ee3b6cad0">foo</div>
<div class="bar">bar </div>
<div>baz</div>
  1. Add a "baz" style to the latest div and save the template file
// hbs
<div class="foo">foo</div>
<div class="bar">bar </div>
<div class="baz">baz</div>
  1. Observe the rendered HTML after build to see there has been no change to the scoped class definitions.
// html output
<div class="foo_ee3b6cad0">foo</div>
<div class="bar">bar </div>
<div class="baz">baz</div>
  1. Update the CSS file with a style definition for bar
// css
.foo {
 	color: red;
 }
 .bar {
 	color: green;
 }
.baz {
	color: blue;
}
  1. Observe the rendered HTML after build to see there has been no change to the scoped class definitions.
// html output
<div class="foo_ee3b6cad0">foo</div>
<div class="bar">bar </div>
<div class="baz">baz</div>

18. Add a "foo" style to the last div and save the template file
```hbs
// hbs
<div class="foo">foo</div>
<div class="bar">bar </div>
<div class="baz foo">baz</div>
  1. Observe the rendered HTML after build to see there has been no change to the second scoped class definition, but the third has updated.
// html output
<div class="foo_ee3b6cad0">foo</div>
<div class="bar">bar </div>
<div class="baz_ee3b6cad0 foo_ee3b6cad0">baz </div>
  1. Add a second "bar" style to the second div and save the template file
// hbs
<div class="foo">foo</div>
<div class="bar bar">bar </div>
<div class="baz foo">baz</div>
  1. Observe the rendered HTML after build to see there has been no change to the scoped class definitions.
// html output
<div class="foo_ee3b6cad0">foo</div>
<div class="bar bar">bar </div>
<div class="baz_ee3b6cad0 foo_ee3b6cad0">baz </div>
  1. Add a fourth div with a "bar" style and save the template file
// hbs
<div class="foo">foo</div>
<div class="bar bar">bar </div>
<div class="baz foo">baz</div>
<div class="bar">biz</div>
  1. Observe the rendered HTML after build to see there has been no change to the second scoped class definitions, but the new fourth div has generated a scoped definition.
// html output
<div class="foo_ee3b6cad0">foo</div>
<div class="bar bar">bar </div>
<div class="baz_ee3b6cad0 foo_ee3b6cad0">baz </div>
<div class="bar_ee3b6cad0">biz</div>

The second div with bar never updates, even though a newer div with the same style does. Definitely appears to be some sort of caching issue, hopefully these steps help!

@jakebixby
Copy link
Contributor Author

jakebixby commented Aug 2, 2023

Something else to note, if you redo the first few steps without adding another div, but adding a second class to the second div, the scoped class is applied. But then if you remove the duplicate class name, the element returns back to the previous state with no scoped class. I'm assuming it's likely a cached key issue (with the single "bar" class):

  1. Create a template file containing a div with a single style
// hbs
<div class="foo">foo</div>
  1. Create an accompanying css file with the style for the div
// css
.foo {
 	color: red;
 }
  1. Observe the rendered HTML does not have a scoped class name for foo
// html output
<div class="foo">foo</div>
  1. Update the template file with a second div with a second single style
// hbs
<div class="foo">foo</div>
<div class="bar">bar</div>
  1. Observe the rendered HTML after build has applied the scoped class name for foo
// html output
<div class="foo_ee3b6cad0">foo</div>
<div class="bar">bar</div>
  1. Update the CSS file with a style definition for bar
// css
.foo {
 	color: red;
 }
 .bar {
 	color: green;
 }
  1. Observe the rendered HTML after build to see there has been no change to the scoped class definitions.
// html output
<div class="foo_ee3b6cad0">foo</div>
<div class="bar">bar</div>
  1. Add a second "bar" class to the second div and save the template file
// hbs
<div class="foo">foo</div>
<div class="bar bar">bar</div>
  1. Observe the rendered HTML after build to see the scoped class definitions applied.
// html output
<div class="foo_ee3b6cad0">foo</div>
<div class="bar_ee3b6cad0 bar_ee3b6cad0">bar</div>
  1. Remove the second "bar" class from the second div and save the template file
// hbs
<div class="foo">foo</div>
<div class="bar">bar</div>
  1. Observe the rendered HTML after build to see the bar scoped class definition has reverted.
// html output
<div class="foo_ee3b6cad0">foo</div>
<div class="bar">bar</div>

Definitely appears to be cache key related, if you revert an element to the state it was before a class was defined, it undoes the scoped class, essentially reverting to the previous state when the class didn’t exist.

@jakebixby
Copy link
Contributor Author

jakebixby commented Aug 16, 2023

After running into the issue again, I decided to do some digging and discovered that part of the issue is that the babel plugin only runs on hbs changes. There may be some order of operations happening with making the hbs change first before the class exists. Adding the class to the css doesn't run the babel plugin, and finally trying to resave the hbs file doesn't re-trigger the babel plugin. I am assuming that is due to some caching, which I have verified is happening by returning the file to a state that had previously been processed. Making a new change (that has never been saved and processed) and saving the hbs file will eventually retrigger the scoped-babel-plugin.

So ultimately, the issue appears to be:

  1. Changes to css files do not trigger the scoped-babel-plugin
  2. Changes to hbs files that have already been processed by the babel plugin do not run again, which can cause a state where a css class will never be picked up unless the hbs file has a change that has not been processed by babel.

I'm not sure exactly what the answer is, but that appears to be the source of the issue.

NullVoxPopuli added a commit that referenced this issue Sep 18, 2023
fixes #90 by adding postProcess hook to scoped-css-preprocessor to break cache for associated template files
@NullVoxPopuli NullVoxPopuli pinned this issue Jan 24, 2024
@NullVoxPopuli
Copy link
Collaborator

Pinning and re-opening because the implemented solutions has caused some confusion, and the problem goes away if we adopt a solution similer to https://github.com/cardstack/glimmer-scoped-css (have everything in one file)

@NullVoxPopuli NullVoxPopuli reopened this Jan 24, 2024
@NullVoxPopuli
Copy link
Collaborator

NullVoxPopuli commented Feb 14, 2024

A way to solve this (that we talked about at the onsite), was being explored here, but we can't currently pursue that implementation because it would mean we lose HOT loading of CSS, which is a requirement for us internally due to how numerous our dozens of app MB are.

We may have to

  • give up on hot loading of CSS (to potentially solve this problem)
    (this is because the CSS hot loading technique we use now is dependent on the legacy style pipeline, and we wouldn't be using that with "the fix" -- we'd make all CSS part of the module graph)
    ((I could be wrong here, and this may still need more exploration (and it's still worth doing that exploration, as we'll want a more robust solution when we get to vite anyway, something that plays nicer with "compile only what you see" --- but for now, I'm deprioritizing this exploration)))
  • or migrate to embroider/vite all at once (maybe via flag so folks can beta test)

Right now, our scoped-css approach is at risk because we don't have parity between embroider and non-embroider. See here for details #120
We can re-gain that parity, but it'll just take some time.

@NullVoxPopuli NullVoxPopuli unpinned this issue Feb 14, 2024
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