css-blocks icon indicating copy to clipboard operation
css-blocks copied to clipboard

Error with inherited state in htmlbars template

Open dacoho opened this issue 7 years ago • 8 comments

I get the following error with the Ember addon when I try to extend a block with an state:

TemplateError: Can not apply multiple states at the same time from the exclusive state group ":scope[state|error]"

/* block-1.block.css */
:scope[state|error] {
    ...
}

/* block-2.block.css */
@block-reference block1 from "block-1.block.css";
:scope {
    extends: block1
}

/* block-2.hbs */
<div state:error={{error}}></div>

When the state is not inherited, the build is fine:

/* block-1.block.css */
:scope[state|error] {
   ...
}

/* block-1.hbs */
<div state:error={{error}}></div>

dacoho avatar Oct 03 '18 15:10 dacoho

Hm. Thats interesting. Will take a look asap, but it may be another week before I can get it on my docket. Think you could pop up a branch with a failing test case?

amiller-gh avatar Oct 09 '18 20:10 amiller-gh

Sure! This case fails: https://github.com/dacoho/css-blocks/commit/b88d28caf7b4c5620a427bd8093a9968cd6025d9

dacoho avatar Oct 11 '18 15:10 dacoho

Amazing 🎉 Now that I'm back from the Node.js collab summit I'll dig in to this a bit.

amiller-gh avatar Oct 15 '18 17:10 amiller-gh

Hi @amiller-gh, I've been reviewing this and have found that in the glimmer analysis, inherited static and dynamic attributes are being added to each element several times - once for each of the block classes.

That happens in this loop: https://github.com/linkedin/css-blocks/blob/7079a5320f2b29f31e246c0c1476de40af35512c/packages/%40css-blocks/glimmer/src/ElementAnalyzer.ts#L218

When attributes added only once the bug disappears. e.g.

let applied = analysis.element.addedStyles.find(style => style.value == state);
if (!applied) {
    analysis.element.addDynamicAttr(container, state, null);
}

Would this be a good solution?

dacoho avatar Oct 18 '18 06:10 dacoho

Hey @dacoho, sorry for the slow reply – great sleuthing. That seems like a solid solution, but can we maybe add the existence check to addDynamicAttr so Analyzers don't need to worry about this logic? My hope is for Analysis integration to be as straightforward as possible, if we already made this mistake in one of our own analyzers then it sounds like something that @css-blocks/core can help validate for us!

We can keep a faster model for style presence check on ElementAnalysis instead of iterating through addedStyles each time. addDynamicAttr (as well as all the other style application methods: addDynamicClasses, addStaticClass, addDynamicGroup, addStaticAttr) would use that to check if a Class or Attr has already been added. If yes, no-op, if no, add the Style(s).

amiller-gh avatar Nov 06 '18 14:11 amiller-gh

I'm seeing an error with states and Glimmer component in an ember-engine that may be related.

Cannot select attributes other than states: :scope[is-on]

Component block CSS

:scope {
    font-weight: bold;
}

:scope[is-on] {
    color: red;
}

Glimmer Component hbs.

<button block:scope block:is-on={{isOn}} {{on "click" this.toggleOn}}>
    {{yield}}
</button>
  • "@css-blocks/ember-cli": "^0.24.0"
  • "ember-cli": "~3.14.0"
  • "ember-engines": "^0.8.3"
  • "@glimmer/component": "1.0.0-beta.3"

GCheung55 avatar Dec 05 '19 00:12 GCheung55

@GCheung55 You're using the new syntax for 1.0 but the version of css-blocks that you've got installed is 0.24. You can either install @css-blocks/ember-cli@next or use the older syntax in your glimmer templates. I'm working on a new release for @next with a lot of bug fixes right now... it should be out in the next day or two.

chriseppstein avatar Dec 07 '19 08:12 chriseppstein

@chriseppstein switching to 1.0 worked for me. Thanks!

GCheung55 avatar Dec 11 '19 06:12 GCheung55