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

Re-organize @stylable/core #1991

Open
16 of 33 tasks
tomrav opened this issue Aug 8, 2021 · 1 comment
Open
16 of 33 tasks

Re-organize @stylable/core #1991

tomrav opened this issue Aug 8, 2021 · 1 comment
Assignees
Labels
core Processing and transforming logic dev velocity plan a plan for organizing larger amounts of work tech debt Updates, upgrades, stale code and work-arounds

Comments

@tomrav
Copy link
Collaborator

tomrav commented Aug 8, 2021

description

organize @stylable/core in order to achieve improved code flow that is intuitive and predictable for searching implementation and tests.

The issues we face today while developing Stylable:

  • tests are organized in a way that is hard to predict which causes duplications and unknowns
  • source is mostly inlined in the 2 big processes (processor & transformer) with constraints that are difficult to understand between the inner processes
  • source is placed in a flat structure with lots of messy generic modules

blocked by (dependencies)

tasks overview

System to manage features

The main issue with testing e2e "features" in Stylable is the fact that they are interleaved in implementation, tests and usage. That makes it impossible to arbitrary decide how to breakdown specs and where a feature should be tested.

I suggest we keep an ordered list of features and make sure that tests are written in the lowest feature spec that is being tested. For example if a test uses @st-import and @property and custom properties is first on the list, then the test would be under the st-import.spec.

  • place feature specs under test/features
  • features will be grouped into 3 types:
    • base - are low level features that statically register types and handleing of higher level features
    • extend css - CSS features that are handled or extended by Stylable
    • stylable - pure stylable additions to the language
  • other unit tests can be placed inlined with their source path
  • feature spec should be organized with optional test groups:
    • meta - data that is kept on the meta and public / private API to interact with it
    • transform - final AST output
    • diagnostics - output with destination between analyze and transform phases

breakdown the big process

  • split processor / transformer into smaller phases
  • extract implementation according to "features" (similar to tests) src/features
    • feature is organized according to new process phases
    • feature contain the set of warn/error and a single warn/error of all features is combined and used in tests
  • break down any utils into semantic helper modules src/helpers
  • pass a single global context across for configuration
  • keep new api private by default - open public API on StylableMeta just for known required usages
  • remove all transformations from the current processor flow

helpers

  • resolve
  • escape
  • deprecation
  • rule
  • selector
  • declaration
  • global

context

passed across the entire process

  • mode - production|development
  • [future] presetEnv - decide on build target / polyfills
  • parsers
    • cssParser
    • selectorParser
    • valueParser

process breakdown suggestion

New flow will have a cleaner StylableMeta that will hold private states for the analyze and transform flows

analyzeStylesheet (previously stylable-processor)

Analyze a single stylesheet into a state that can be cached against the content of the stylesheet.

  • create empty StylableMeta
  • analyzeInit
    • st-import - collect import symbols - check symbols re-declaration, default, named, keyframes (need to discuss) // ToDo: deprecate :import
  • iterate at-rules / rules
    • parse rule.selector | atRule.params
    • analyzeAtRule<NAME>
      • st-namespace - collect
      • css-keyframes - collect
      • css-custom-selector - collect
      • st-scope - collect // ToDo: move validation here
      • css-property - collect + check redeclare symbol (@st-global-custom-property)
    • analyzeSelectorNode<AST_TYPE, VALUE> - walk selector AST
      • st-import - check deprecated syntax :import
      • st-var - check (soon to be) deprecated syntax :vars
      • st-part - add part symbol
      • st-part - add type symbol, alias
      • css-id - validate no functional api
      • css-class - collect class symbols
      • css-attribute - validate no functional api
      • css-nesting - validate no functional api
    • analyzeRule
      • st-part - validate root position in selector
      • css-nesting - unsupported - check no nested rules
  • iterate declarations
    • parse declaration.value (once)
    • analyzeDeclaration<PROP>
      • st-class - collect -st-states
      • st-class - collect -st-extends
      • st-class - add to -st-global
      • st-mixin/partialMixin // ToDo: stop setting on Rule
      • css-property - collect css var (set)
    • analyzeDeclarationValue<PART> - walk value AST
      • css-property - collect var() (get)
      • css-asset - collect url() // ToDo: check if required (might be doubled checked in transformer because of mixins)
  • onAnalyzeComplete
    • st-custom-selector - temp keep removing rule
    • css-property - temp keep removing the rule (@st-global-custom-property)
    • st-scope - merge scoped selectors // ToDo: stop changing source AST
    • st-var - temp keep removing :vars
    • st-import - temp keep removing :import / @import
    • st-namespace - set namespace on meta + temp keep removing rule // might be part of createStylesheetMeta and not the analyze

transformStylesheet (previously stylable-transformer)

Picks up after the meta has been created and when dependencies are available, transform the sourceAST into targetAST while adding diagnostics.

  • clone sourceAST into targetAST
  • transformInit
    • global
    • st-scope validate scopes keep track of scopes // ToDo: remove as part of not flattening @st-scope in the processor
    • st-import - validate imports
    • css-var - collect local & imported vars
  • iterate at-rules / rules
    • parse rule.selector | atRule.params
    • transformSelector - transform rule selector + keep track of nested selector type
    • transformAtRule<NAME>
      • css-keyframes - namespace keyframes name
      • css-media - transform inner value() function in params
      • css-property - namespace property
      • st-scope - [future] transform selector & keep track of selector subject type (to set as "anchor" for inner rules)
  • transformDeclaration
  • transformExpandRules - or maybe afterRulesTransform
    • st-class - add dev time browser placement invalidation for -st-extends
    • st-mixin - expand mixin into AST // ToDo: check if the removal of -st-mixin can be delayed to transformCleanup
  • transformCleanup // this might not be a feature phase, but a place that removes AST nodes that have been registered throughout the process
    • st-global - collect :global() selectors and flatten selectors
    • st-mixin - remove mixin declarations?
    • * - remove any other stylable build instructions
  • getJsExports
    • st-part
    • st-var
    • css-keyframes
    • css-property

transformSelector

accepts selector + initial selector context, transform and return the type information at the end of the selector

  • parse selector
  • iterate selector list, iterate selector shallow AST
    • transformSelectorNode<AST_TYPE, VALUE> - iterate selector
      • css-class - namespace & set context for class // ToDo: also validates -st-states here, need to check if there is a better place
      • resolve reference of pseudo_elements
        • css-custom-selector - // ToDo: research process
        • st-part - namespace & set context for custom
      • css-type - namespace & set context non native type
      • css-pseudo-class - transform functional inner selectors & namespace custom state
      • css-nesting - set context to origin // ToDo: check cases when this is required
  • transformRule
    • st-custom-selector - generate multiple selectors if needed

transformDeclaration

  • parse declaration.value (once)
  • transformDeclaration - iterate declarations
    • css-keyframes - namespace animation / animation name
    • css-property - namespace properties in prop
  • transformDeclarationValue<PROP, PART> - walk value AST
    • st-var - transform value() function in declarations value (including custom vars like st-map, st-array, etc)
    • css-asset
    • css-font - handle format() in @font-face
    • st-formatter
    • css-property - transform var()

tasks

@tomrav tomrav added core Processing and transforming logic dev velocity research research for future tasks tech debt Updates, upgrades, stale code and work-arounds :size(large) Effort estimate: large labels Aug 8, 2021
@idoros
Copy link
Collaborator

idoros commented Dec 13, 2021

A collection of bugs or unwanted behaviors related to features that needs to be refactored (WIP):

st-extends

apply to multiple class simple selectors

reported in issue #77, applying a valid -st-extends to multiple simple class selectors could work, but currently fails to extend any part of the selector and failure is only noticed once creating a selector into a custom pseudo-element. see reproduction.

wrong diagnostic for alias with -st-extends

reported in issue #1792, using -st-extends on an imported alias element type can cause a conflict between being an imported alias and a local class that extends according to the -st-extends value.

Since the current behavior already overrides with the local definition, which matched our agreed local override behavior, The issue can be resolved by reporting an Import declaration conflicts with local declaration "${name}"


st-global

Override behavior

Re-declaring -st-global on the same class take the last definition correctly, but miss the redeclare symbol (or maybe redeclare definition) diagnostics. see reproduction.


css-pseudo-element

pseudo-element of extended root

reported in issue #1960, pseudo-elements that -st-extends a root class are transformed wrongly not composed to the previous part of the selector. see reproduction.


st-mixin

-st-global is not registered to meta

reported in issue #2154 - need to research/expand

handle invalid Javascript mixin declaration

reported issue #1953, the build breaks with an unhelpful message: /entry.st.css:12:34: Missed semicolon when the mixin returns an invalid CSS, see reproduction. handle error and add missing diagnostics for invalid Javascript mixin declaration.

@tomrav tomrav added this to Stylable Dec 14, 2021
@tomrav tomrav moved this to Active in Stylable Dec 14, 2021
@tomrav tomrav added plan a plan for organizing larger amounts of work and removed research research for future tasks :size(large) Effort estimate: large labels Dec 19, 2021
@tomrav tomrav removed the status in Stylable Jan 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Processing and transforming logic dev velocity plan a plan for organizing larger amounts of work tech debt Updates, upgrades, stale code and work-arounds
Projects
Status: No status
Development

No branches or pull requests

3 participants