feat: Require the :scope pseudo for root states. #94
Conversation
ARCHITECTURE.md
Outdated
| @@ -175,7 +175,7 @@ We can easily conceptualize the `RewriteMapping` data for each element in develo | |||
| ```javascript | |||
| // For Element 1: | |||
| // - `.class-0` is always applied | |||
| // - [state|active] is *only* applied when `isActive` is true | |||
| // - `:scope[state|active]` is *only* applied when `isActive` is true | |||
There was a problem hiding this comment.
Shouldn't this be .class-0[state|active]?
| */ | ||
| export function isExternalBlock(object: NodeAndType): boolean { | ||
| return object.blockType === BlockType.block; | ||
| } |
There was a problem hiding this comment.
We should name the anonymous interfaces in NodeAndType and then this (and the other related isXXX checks) should become typeguards.
There was a problem hiding this comment.
Hmmmmkay refactored 👍 (Even though its going away...soon...hopefully...)
| throw new errors.InvalidBlockSyntax( | ||
| `It's redundant to specify a state with an explicit .root: ${rule.selector}`, | ||
| loc(file, rule, found.node), | ||
| `States without an explicit :scope or class selector are not yet supported: ${rule.selector}`, |
There was a problem hiding this comment.
don't say "yet" here. It may not happen 🤷♂️
| } | ||
| throw new errors.InvalidBlockSyntax( | ||
| `Missing global state selector on external Block '${result.node.value}'. Did you mean one of: ${globalStates.map((s) => s.asSource()).join(" ")}`, | ||
| loc(file, rule, sel.nodes[0])); |
There was a problem hiding this comment.
There should be an error for when the state with that name exists, but it's not global.
| @@ -33,11 +33,52 @@ export class BlockInheritance extends BEMProcessor { | |||
| ); | |||
| }); | |||
| } | |||
| @test "Global state usage must be specify a global state if present"() { | |||
There was a problem hiding this comment.
"must be specify" => "must be able to specify"?
| @@ -198,7 +198,7 @@ export class Test { | |||
| ).then((analyzer: Analyzer) => { | |||
| let result = analyzer.serialize(); | |||
| let analysis = result.analyses[0]; | |||
| assert.deepEqual(analysis.stylesFound, ["bar.pretty", "bar.pretty[state|awesome]", "bar:scope", "bar[state|awesome]"]); | |||
| assert.deepEqual(analysis.stylesFound, ["bar.pretty", "bar.pretty[state|awesome]", "bar:scope", "bar:scope[state|awesome]"]); | |||
There was a problem hiding this comment.
bar:scope I don't think we should ever use :scope in combination with a block name.
| block-b[state|bar=two] (blocks/b.block.css:4:40)`, | ||
| block-a:scope[state|foo=one] (blocks/foo.block.css:4:46) | ||
| block-b:scope[state|bar=one] (blocks/b.block.css:3:43) | ||
| block-b:scope[state|bar=two] (blocks/b.block.css:4:46)`, |
There was a problem hiding this comment.
Weith an explicit block name the :scope is redundant. I think the scope selector is best reserved for inside the block file where the name of the block is a suggestion at best.
- All local types have been pulled out to the project directory. - Packages moved to /packages/@css-blocks for module name generation. - typedoc.js configuration file added. - packages/css-blocks => packages/@css-blocks/core - packages/webpack-plugin => packages/@css-blocks/webpack. - packages/glimmer-templates => packages/@css-blocks/glimmer. - packages/runtime/src/runtime.ts => packages/@css-blocks/runtime/src/index.ts. - yarn run docs will generate a typedoc site at /docs. - TODO: Uses pre-built forked version of typedoc-plugin-external-module-map. Remove when merged.
- Buggy getBlockNode removed from block-intermediates - block-intermediates made a private module of BlockParser - Unrelated: Fix webpack plugin file watcher for auto rebuilds
amiller-gh
force-pushed
the
require-scope
branch
from
d06e9d5 to
aa523f8
Compare
block-intermediatesmade a private utility ofBlockParserStill TODO (at a later date): This PR starts to clean up the distinctions between
block-intermediatesand,BlockPathparser and theAttrTokeninterface, but there is still more to do! Step in the right direction.