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

fix(core): st-var feature overall #2284

Merged
merged 17 commits into from
Feb 8, 2022
Merged

Conversation

idoros
Copy link
Collaborator

@idoros idoros commented Jan 23, 2022

fix

  • testStylableCore doesn't check inline diagnostics in nodes that are removed by processor
  • warn on missing arguments to value() function
  • JS export quotes not removed - playground
  • using imported class vs local class as variable doesn't result in the same error - playground
  • unhandled crush when importing unknown symbol using @st-import - playground

refactor

  • processor
    • analyzeSelectorNode - move definition/validation to feature
  • transform
    • transformDeclarationValue - move value() transformation to feature
    • transformJSExports - move to feature

deprecate

  • meta.vars

test

  • move tests to st-var.spec and convert to new format
  • add missing tests

move to backlog

  • CSS custom property not transformed correctly in @media and :vars #2318
  • value evaluation and custom-value mechanism is broken for many cases - need refactor:
    • refactor
      • change collectUrls in analyze to run just in walk decls by removing :vars at the end
      • move custom-value transform evaluation to feature
    • fix
      • st-map and st-array output unuseful string when accessed at top level - playground
      • concat multiple custom values in definition is broken with no error - playground
      • nested st-map reference doesn't work (st-array works fine) - playground
      • locally assigned custom value from imported var cannot use inner path to access value and reports error - playground
  • evaluate var in definition context only - playground
    • value with CSS custom property
    • value in quotation
    • value from formatter
    • value from JS import

@idoros idoros added core Processing and transforming logic tech debt Updates, upgrades, stale code and work-arounds labels Jan 23, 2022
@idoros idoros self-assigned this Jan 23, 2022
@idoros idoros mentioned this pull request Jan 23, 2022
33 tasks
- init feature in meta
- move `VarSymbol` to feature
- move processor code to feature
- add walk return type to `analyzeSelectorNode` to alllow walk control
- add `evaluator` to `FeatureTransformContext`
- resolve local vars in feature
- transform JS exports in feature
- change Evaluator to stateful class to hold `tsVarOverride` data

- create evaluator per transformer
- remove `transformDeclarationValue` resolved data - only used by CSS custom property and already exist in data interface
- support inline diagnostics check for `:vars`
- support inline tests for non transformed stylesheets
- add `STVar.get()` to get symbol from feature
- move tests from various specs to feature
- added simple missing expectations
- added noticable ToDos for unexpected behavior
packages/core/src/functions.ts Outdated Show resolved Hide resolved
packages/core/src/stylable-transformer.ts Outdated Show resolved Hide resolved
packages/core/src/stylable-transformer.ts Outdated Show resolved Hide resolved
packages/core/src/stylable-transformer.ts Show resolved Hide resolved
packages/core/test/features/st-var.spec.ts Outdated Show resolved Hide resolved
packages/core/test/features/st-var.spec.ts Show resolved Hide resolved
packages/core/test/functions.spec.ts Show resolved Hide resolved
@idoros idoros changed the title refactor(core): st-var feature extraction fix(core): st-var feature overall Feb 7, 2022
- previously worked in legacy `:import` while crushing in `@st-import`
- change `transformDeclarationValue` to `transformValue`
- moved `Evaluator` initialization to head
- changes `Evaluator` to accept optional options
- add missed test for access custom value using path with errors
- change named error to be generic as it is not just used by mixins
- remove symbol value mapping from st-var feature: call fields directly
- move string functions to `helpers/string`
- replaced `deprecated` util with `warnOnce` from `helpers/deprecate`
@barak007 barak007 self-requested a review February 8, 2022 12:42
- removed ToDo comment that was done
- moved private the end of `transformer` head
@idoros idoros merged commit a34a623 into master Feb 8, 2022
@idoros idoros deleted the ido/st-var-feature-extraction branch February 8, 2022 13:28
@idoros idoros added the bug Unexpected behavior or exception label Feb 15, 2022
tzachbon pushed a commit that referenced this pull request Feb 21, 2022
## fix
- warn on missing arguments to `value()` function
- remove outer quotes from Stylable vars JS exports
- unify error when using imported class and local class
- handle unknown symbol using `@st-import` - previously caused a crush

## deprecate
- `meta.vars` - use `meta.getAllStVars()` or `meta.getStVar(name)` instead
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unexpected behavior or exception core Processing and transforming logic tech debt Updates, upgrades, stale code and work-arounds
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

None yet

2 participants