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

feat: strict custom property #2929

Merged
merged 7 commits into from
Dec 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions packages/cli/src/config/projects-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +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 { processNamespace } from '@stylable/core';
import type { StylableConfig } from '@stylable/core';
import type { IFileSystem } from '@file-services/types';

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 @@ -108,9 +109,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 @@ -273,8 +279,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 @@ -22,6 +22,7 @@ import { STImport, STScope, STVar, STMixin, CSSClass, CSSCustomProperty } from '
import { Dependency, visitMetaCSSDependencies } from './visit-meta-css-dependencies';
import * as postcss from 'postcss';
import { warnOnce } from './helpers/deprecation';
import { defaultFeatureFlags, type FeatureFlags } from './features/feature';

export interface StylableConfigBase {
projectRoot: string;
Expand All @@ -35,6 +36,7 @@ export interface StylableConfigBase {
resolverCache?: StylableResolverCache;
fileProcessorCache?: Record<string, CacheItem<StylableMeta>>;
experimentalSelectorInference?: boolean;
flags?: Partial<FeatureFlags>;
}

export type StylableConfig = StylableConfigBase &
Expand All @@ -53,6 +55,8 @@ export type StylableConfig = StylableConfigBase &
const globalDefaultSupportedConfigs = new Set([
'resolveModule',
'resolveNamespace',
'requireModule',
'flags',
'experimentalSelectorInference',
]);
export function validateDefaultConfig(defaultConfigObj: any) {
Expand Down Expand Up @@ -98,6 +102,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 === false ? false : true;
Expand All @@ -124,12 +129,17 @@ export class Stylable {
this.cssParser = config.cssParser || cssParse;
this.resolverCache = config.resolverCache || 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 @@ -179,7 +189,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 @@ -290,6 +290,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 @@ -1038,8 +1076,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 @@ -1049,7 +1088,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 @@ -1063,7 +1102,9 @@ describe(`features/css-custom-property`, () => {
--c: var(--c);
}
`,
});
},
{ stylableConfig: { flags: { strictCustomProperty: true } } }
);

const { meta: nativeMeta } = stylable.transform('/native.css');
const { meta } = stylable.transform('/entry.st.css');
Expand All @@ -1084,6 +1125,23 @@ describe(`features/css-custom-property`, () => {
`)
);
});
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