Skip to content

Commit

Permalink
feat: strict custom property (#2929)
Browse files Browse the repository at this point in the history
  • Loading branch information
idoros committed Dec 3, 2023
1 parent 3841888 commit 048fee2
Show file tree
Hide file tree
Showing 10 changed files with 132 additions and 22 deletions.
16 changes: 10 additions & 6 deletions packages/cli/src/config/projects-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,19 @@ import type {
import { processProjects } from './process-projects';
import { createDefaultOptions, mergeBuildOptions, validateOptions } from './resolve-options';
import { resolveNpmRequests } from './resolve-requests';
import type { ModuleResolver } from '@stylable/core/dist/index-internal';
import type { MinimalFS, processNamespace } from '@stylable/core';
import type { MinimalFS } from '@stylable/core';
import type { StylableConfig } from '@stylable/core';

interface StylableRuntimeConfigs {
stcConfig?: Configuration<string> | undefined;
defaultConfig?: {
resolveModule?: ModuleResolver;
resolveNamespace?: typeof processNamespace;
};
defaultConfig?: Pick<
StylableConfig,
| 'resolveNamespace'
| 'requireModule'
| 'resolveModule'
| 'flags'
| 'experimentalSelectorInference'
>;
}

export async function projectsConfig(
Expand Down
7 changes: 4 additions & 3 deletions packages/core-test-kit/src/generate-test-util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
createStylableFileProcessor,
postProcessor,
replaceValueHook,
defaultFeatureFlags,
} from '@stylable/core/dist/index-internal';
import { isAbsolute } from 'path';
import * as postcss from 'postcss';
Expand Down Expand Up @@ -107,9 +108,9 @@ export function processSource(
options: { from?: string } = {},
resolveNamespace?: typeof processNamespace
) {
return new StylableProcessor(new Diagnostics(), resolveNamespace).process(
postcss.parse(source, options)
);
return new StylableProcessor(new Diagnostics(), resolveNamespace, {
...defaultFeatureFlags,
}).process(postcss.parse(source, options));
}

export function createProcess(
Expand Down
11 changes: 8 additions & 3 deletions packages/core/src/create-stylable-processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { cssParse, CssParser } from './parser';
import { processNamespace, StylableProcessor } from './stylable-processor';
import type { StylableMeta } from './stylable-meta';
import type { Diagnostics } from './diagnostics';
import { defaultFeatureFlags, type FeatureFlags } from './features/feature';

export function createStylableFileProcessor({
fileSystem,
Expand All @@ -11,8 +12,10 @@ export function createStylableFileProcessor({
cssParser = cssParse,
cache,
createDiagnostics,
flags = { ...defaultFeatureFlags },
}: {
fileSystem: MinimalFS;
flags?: FeatureFlags;
onProcess?: (meta: StylableMeta, path: string) => StylableMeta;
resolveNamespace?: typeof processNamespace;
cssParser?: CssParser;
Expand All @@ -21,9 +24,11 @@ export function createStylableFileProcessor({
}) {
return cachedProcessFile<StylableMeta>(
(from, content) => {
return new StylableProcessor(createDiagnostics?.(from), resolveNamespace).process(
cssParser(content, { from })
);
return new StylableProcessor(
createDiagnostics?.(from),
resolveNamespace,
flags
).process(cssParser(content, { from }));
},
(resolvedPath: string) => {
return fileSystem.readFileSync(resolvedPath, 'utf8');
Expand Down
21 changes: 19 additions & 2 deletions packages/core/src/features/css-custom-property.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ export const diagnostics = {
'error',
() => `missing custom property name for "var(--[PROP NAME])"`
),
UNDEFINED_CSS_CUSTOM_PROP: createDiagnosticReporter(
'01011',
'error',
(name) =>
`Undefined "${name}" custom property. Please define the property using '@property' or import it with '@st-import' when 'strictCustomProperty' is enabled.`
),
};

const dataKey = plugableRecord.key<{
Expand Down Expand Up @@ -248,8 +254,19 @@ function addCSSProperty({
return;
}
// usages bailout: addition of weak definition reference `--x: var(--x)`
if (!final && !!STSymbol.get(context.meta, name, `cssVar`)) {
return;
if (!final) {
const existing = STSymbol.get(context.meta, name, `cssVar`);
if (existing) {
// already defined
return;
// eslint-disable-next-line no-constant-condition
} else if (context.meta.type === 'stylable' && context.meta.flags.strictCustomProperty) {
// strict mode
context.diagnostics.report(diagnostics.UNDEFINED_CSS_CUSTOM_PROP(name), {
node,
word: name,
});
}
}

// define symbol
Expand Down
7 changes: 7 additions & 0 deletions packages/core/src/features/feature.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@ import type { ImmutableSelectorNode } from '@tokey/css-selector-parser';
import type { Diagnostics } from '../diagnostics';
import type { ParsedValue } from '../types';

export interface FeatureFlags {
strictCustomProperty: boolean;
}
export const defaultFeatureFlags: FeatureFlags = {
strictCustomProperty: false,
};

export type SelectorNodeContext = [
index: number,
nodes: ImmutableSelectorNode[],
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/index-internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export {
CSSCustomProperty,
STStructure,
} from './features';
export { defaultFeatureFlags } from './features/feature';
export type {
MappedStates,
StateParsedValue,
Expand Down
7 changes: 6 additions & 1 deletion packages/core/src/stylable-meta.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
CSSContains,
STStructure,
} from './features';
import type { FeatureFlags } from './features/feature';

const features = [
STSymbol,
Expand Down Expand Up @@ -61,7 +62,11 @@ export class StylableMeta {
// Generated during transform
public targetAst?: postcss.Root;
public globals: Record<string, boolean> = {};
constructor(public sourceAst: postcss.Root, public diagnostics: Diagnostics) {
constructor(
public sourceAst: postcss.Root,
public diagnostics: Diagnostics,
public flags: FeatureFlags
) {
// initiate features
const context: FeatureContext = { meta: this, diagnostics };
for (const { hooks } of features) {
Expand Down
6 changes: 4 additions & 2 deletions packages/core/src/stylable-processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,17 @@ import {
stringifySelector,
} from './helpers/selector';
import { isChildOfAtRule } from './helpers/rule';
import { defaultFeatureFlags, type FeatureFlags } from './features/feature';

export class StylableProcessor implements FeatureContext {
public meta!: StylableMeta;
constructor(
public diagnostics = new Diagnostics(),
private resolveNamespace = STNamespace.defaultProcessNamespace
private resolveNamespace = STNamespace.defaultProcessNamespace,
public flags: FeatureFlags = { ...defaultFeatureFlags }
) {}
public process(root: postcss.Root): StylableMeta {
this.meta = new StylableMeta(root, this.diagnostics);
this.meta = new StylableMeta(root, this.diagnostics, this.flags);

STStructure.hooks.analyzeInit(this);
STImport.hooks.analyzeInit(this);
Expand Down
12 changes: 11 additions & 1 deletion packages/core/src/stylable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { createDefaultResolver } from './module-resolver';
import { STImport, STScope, STVar, STMixin, CSSClass, CSSCustomProperty } from './features';
import { Dependency, visitMetaCSSDependencies } from './visit-meta-css-dependencies';
import * as postcss from 'postcss';
import { defaultFeatureFlags, type FeatureFlags } from './features/feature';

export interface StylableConfig {
projectRoot: string;
Expand All @@ -37,12 +38,15 @@ export interface StylableConfig {
resolverCache?: StylableResolverCache;
fileProcessorCache?: Record<string, CacheItem<StylableMeta>>;
experimentalSelectorInference?: boolean;
flags?: Partial<FeatureFlags>;
}

// This defines and validates known configs for the defaultConfig in 'stylable.config.js
const globalDefaultSupportedConfigs = new Set([
'resolveModule',
'resolveNamespace',
'requireModule',
'flags',
'experimentalSelectorInference',
]);
export function validateDefaultConfig(defaultConfigObj: any) {
Expand Down Expand Up @@ -89,6 +93,7 @@ export class Stylable {
// This cache is fragile and should be fresh if onProcess/resolveNamespace/cssParser is different
protected fileProcessorCache?: Record<string, CacheItem<StylableMeta>>;
private experimentalSelectorInference: boolean;
public flags: FeatureFlags;
constructor(config: StylableConfig) {
this.experimentalSelectorInference = !!config.experimentalSelectorInference;
this.projectRoot = config.projectRoot;
Expand All @@ -109,12 +114,17 @@ export class Stylable {
this.cssParser = config.cssParser || cssParse;
this.resolverCache = config.resolverCache; // ToDo: v5 default to `new Map()`
this.fileProcessorCache = config.fileProcessorCache;
this.flags = {
...defaultFeatureFlags,
...config.flags,
};
this.fileProcessor = createStylableFileProcessor({
fileSystem: this.fileSystem,
onProcess: this.onProcess,
resolveNamespace: this.resolveNamespace,
cssParser: this.cssParser,
cache: this.fileProcessorCache,
flags: this.flags,
});

this.resolver = this.createResolver();
Expand Down Expand Up @@ -154,7 +164,7 @@ export class Stylable {
public createProcessor({
resolveNamespace = this.resolveNamespace,
}: CreateProcessorOptions = {}) {
return new StylableProcessor(new Diagnostics(), resolveNamespace);
return new StylableProcessor(new Diagnostics(), resolveNamespace, this.flags);
}
private createTransformer(options: Partial<TransformerOptions> = {}) {
return new StylableTransformer({
Expand Down
66 changes: 62 additions & 4 deletions packages/core/test/features/css-custom-property.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,44 @@ describe(`features/css-custom-property`, () => {

expect(CSSCustomProperty.getRuntimeTypedDefinitionNames(meta)).to.eql(['--c', '--d']);
});
it('should warn on undefined property in strict mode', () => {
testStylableCore(
{
'/invalid.st.css': `
.root {
/*
@analyze-error word(--prop) ${customPropertyDiagnostics.UNDEFINED_CSS_CUSTOM_PROP(
'--prop'
)}
@decl --invalid-prop: green
*/
--prop: green;
/*
@analyze-error word(--prop2) ${customPropertyDiagnostics.UNDEFINED_CSS_CUSTOM_PROP(
'--prop2'
)}
@decl color: var(--invalid-prop2)
*/
color: var(--prop2);
}
`,
'/valid.st.css': `
@st-import [--prop] from './invalid.st.css';
@property --prop2;
.root {
/* @decl --invalid-prop: green */
--prop: green;
/* @decl color: var(--valid-prop2) */
color: var(--prop2);
}
`,
},
{ stylableConfig: { flags: { strictCustomProperty: true } } }
);
});
it.skip(`should escape`, () => {
const { sheets } = testStylableCore(`
.root {
Expand Down Expand Up @@ -966,8 +1004,9 @@ describe(`features/css-custom-property`, () => {
});
describe('native css', () => {
it('should not namespace', () => {
const { stylable } = testStylableCore({
'/native.css': deindent(`
const { stylable } = testStylableCore(
{
'/native.css': deindent(`
@property --a {
syntax: '<color>';
initial-value: green;
Expand All @@ -977,7 +1016,7 @@ describe(`features/css-custom-property`, () => {
--b: var(--c);
}
`),
'/entry.st.css': `
'/entry.st.css': `
@st-import [--a, --b, --c] from './native.css';
.root {
Expand All @@ -991,7 +1030,9 @@ describe(`features/css-custom-property`, () => {
--c: var(--c);
}
`,
});
},
{ stylableConfig: { flags: { strictCustomProperty: true } } }
);

const { meta: nativeMeta } = stylable.transform('/native.css');
const { meta, exports } = stylable.transform('/entry.st.css');
Expand Down Expand Up @@ -1019,6 +1060,23 @@ describe(`features/css-custom-property`, () => {
c: '--c',
});
});
it('should ignore strictCustomProperty', () => {
const { stylable } = testStylableCore(
{
'/entry.css': `
.root {
/* @decl --a: var(--z) */
--a: var(--z);
}
`,
},
{ stylableConfig: { flags: { strictCustomProperty: true } } }
);

const { meta } = stylable.transform('/entry.css');

shouldReportNoDiagnostics(meta);
});
it('should ignore stylable specific transformations', () => {
const { stylable } = testStylableCore({
'/native.css': deindent(`
Expand Down

0 comments on commit 048fee2

Please sign in to comment.