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): css-custom-property feature overall #2216

Merged
merged 23 commits into from
Jan 23, 2022

Conversation

idoros
Copy link
Collaborator

@idoros idoros commented Dec 21, 2021

fix

  • questioned in @st-global-custom-property and @property interaction #1954 currently @st-global-custom-property overrides the @property definition regardless of the definitions order see behavior - This PR resolves and tests the definition behavior:
    • deprecated with a deprecation info diagnostic
    • redeclare diagnostic when conflicting with @property and mark property as global
    • redeclare diagnostic when conflicting with @st-import and override with global symbol
    • define global property symbol in any other case
  • imported properties are not collected during analyze, and prop/var() usages creates an incorrect symbol - this PR fixes the issue by adding an alias ref to the imported symbol (like class and type symbols), and imported custom properties are identified by the -- prefix and added to as an aliased property
  • empty var() break the process with: cannot read properties of undefined (reading 'type') - this PR keeps var() untransformed, report missing property name diagnostic and doesn't crush
  • noticed on the way
    • fix(core): wrong transform for empty @keyframes {} -> @keyframesNS__ {} see playground

refactor

  • processor
    • simplify the property collection process
    • analyzeAtRule
      • move definition/validation of @property to feature
      • move definition of @st-global-custom-property to feature
    • analyzeDeclaration (new hook)
      • move "declare-unknown" definition for --X: value; to feature
      • move "declare-unknown" definition for prop: var(--X) to feature
  • transformer
    • transformResolve - move createCSSVarsMapping to feature
    • transformAtRule - move @property namespace/removal to feature
    • transformDeclaration - move --X: ; namespace to feature
    • transformDeclarationValue (new hook) - move : var(); namespace to feature (value transformation is part of evelDeclarationValue)
    • transformJSExports - move exportCSSVars to feature
  • small
    • CSSVarSymbol to feature
    • move validate-at-property to helpers/css-custom-property
    • move code to new helpers/global
    • move stylable-utils/isCSSVarProp to helpers/css-custom-property

deprecate

  • meta.cssVars
  • paramMapping export

test

  • move tests to css-property.spec
  • add missing tests
  • fix(core-test-kit): inline expectation @analayze & @transform bug with multiple lines

@idoros idoros added core Processing and transforming logic tech debt Updates, upgrades, stale code and work-arounds labels Dec 21, 2021
@idoros idoros self-assigned this Dec 21, 2021
- feature file `/features/css-custom-property.ts`
- moved `CSSVarSymbol` symbol
- export empty diagnostics and hooks
- register to stylable meta class
- moved all code related to feature file
- new `analyzeDeclaration` feature hook
- `isCSSVarProp` from `stylable-utils` to `helpers/css-custom-property`
- `validateAllowedNodesUntil` from `stylable-value-parser` to `helpers/value`
- `globalValue` from `utils` to `helpers/global`
- deprecate `paramMapping` constant export
- analyze `@st-global-custom-property` to feature
- add `toRemove` api to `analyzeAtRule` feature until `rawAst` is immutable
- `transformResolve`
- `transformAtRule`
- `transformJSExports`
- `transformDeclarationValue` new hook
- transform `var()` extract from `functions` to feaure
- `stringifyFunction` from `functions` to `helpers/value`
- `transformDeclaration` - move `--X: ;` namespace to feature
- `getScopedCSSVar` from `transformer` to `helpers/css-custom-property`
- deprecate `transformer.getScopedCSSVar()`
- refactored to new format with inline tests and `testStylableCore`
- add internal `CSSCustomProperty.get(symbolName)`
- `exports.spec` css custom property tests
- test for resolving mixed-in property override
- moved skipped escape test
- add `alias` field to `CSSVarSymbol` to point to imported symbol
- implemented suppurt for aliased in resolver and completion-provider
- register `--` prefixed imports as aliased custom property symbols
- sort conflicted symbol definitions from import, `@property`, and `@st-custom-global-property`
- import path and file name didn't match
- keep `var()` untransformed
- report empty prop name diagnostic
@idoros idoros linked an issue Jan 19, 2022 that may be closed by this pull request
@idoros idoros mentioned this pull request Jan 19, 2022
33 tasks
@idoros idoros requested a review from barak007 January 23, 2022 07:35
@idoros idoros merged commit a17f422 into master Jan 23, 2022
@idoros idoros deleted the ido/css-custom-property-feature-extraction branch January 23, 2022 08:53
@idoros idoros changed the title refactor(core): css-custom-property feature extraction fix(core): css-custom-property feature overall Feb 8, 2022
tzachbon pushed a commit that referenced this pull request Feb 21, 2022
## fix
- resolve behavior between `@property` and `@st-global-custom-property`
- collect imported properties during analyze (event if not used)
- empty `var()` handle to no crush
- wrong transform for empty `@keyframes {}` -> `@keyframesNS__ {}`

## refactor
- move CSS custom property code to `CSSCustomProperty` feature

## deprecate
- `meta.cssVars`
- `paramMapping` export
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

@st-global-custom-property and @property interaction
2 participants