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.

Sign up for GitHub

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

Jump to bottom

fix: remove root validation #2589

Merged
merged 2 commits into from
Jul 6, 2022
Merged

fix: remove root validation #2589

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
173c349
fix: remove root validation
barak007 Jul 4, 2022
6d942ac
fix: change test subject
barak007 Jul 4, 2022
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
26 changes: 0 additions & 26 deletions packages/core/src/deprecated/deprecated-selector-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,32 +182,6 @@ export function traverseNode(
}
}

export function isRootValid(ast: SelectorAstNode, rootName: string) {
let isValid = true;

traverseNode(ast, (node, index, nodes) => {
if (node.type === 'nested-pseudo-class') {
return true;
}
if (node.type === 'class' && node.name === rootName) {
let isLastScopeGlobal = false;
for (let i = 0; i < index; i++) {
const part = nodes[i];
if (isGlobal(part)) {
isLastScopeGlobal = true;
}
if (part.type === 'spacing' && !isLastScopeGlobal) {
isValid = false;
}
if (part.type === 'element' || (part.type === 'class' && part.value !== 'root')) {
isLastScopeGlobal = false;
}
}
}
return undefined;
});
return isValid;
}
export function isGlobal(node: SelectorAstNode) {
return node.type === 'nested-pseudo-class' && node.name === 'global';
}
Expand Down
31 changes: 0 additions & 31 deletions packages/core/src/helpers/selector.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,37 +127,6 @@ export function matchTypeAndValue(
return a.type === b.type && (a as any).value === (b as any).value;
}

export function isRootValid(ast: ImmutableSelectorList) {
let isValid = true;
walk(ast, (node, index, nodes) => {
if (node.type === 'pseudo_class') {
return walk.skipNested;
}
if (node.type === 'class' && node.value === `root`) {
let isLastScopeGlobal = false;
for (let i = 0; i < index; i++) {
const part = nodes[i];
if (isGlobal(part)) {
isLastScopeGlobal = true;
}
if (part.type === 'combinator' && !isLastScopeGlobal) {
isValid = false;
return walk.skipCurrentSelector;
}
if (part.type === 'type' || (part.type === 'class' && part.value !== 'root')) {
isLastScopeGlobal = false;
}
}
}
return undefined;
});
return isValid;
}

function isGlobal(node: ImmutableSelectorNode) {
return node.type === 'pseudo_class' && node.value === 'global';
}

export function isCompRoot(name: string) {
return name.charAt(0).match(/[A-Z]/);
}
Expand Down
3 changes: 1 addition & 2 deletions packages/core/src/index-deprecated.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,6 @@ import {
matchAtKeyframes as deprecatedMatchAtKeyframes,
isImport as deprecatedIsImport,
isSimpleSelector as deprecatedIsSimpleSelector,
isRootValid as deprecatedIsRootValid,
isGlobal as deprecatedIsGlobal,
createChecker as deprecatedCreateChecker,
isNested as deprecatedIsNested,
Expand Down Expand Up @@ -325,7 +324,7 @@ export const isSimpleSelector = wrapFunctionForDeprecation(deprecatedIsSimpleSel
name: `isSimpleSelector`,
});
/**@deprecated*/
export const isRootValid = wrapFunctionForDeprecation(deprecatedIsRootValid, {
export const isRootValid = wrapFunctionForDeprecation(() => true, {
name: `isRootValid`,
});
/**@deprecated*/
Expand Down
8 changes: 0 additions & 8 deletions packages/core/src/stylable-processor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import {
walkSelector,
isSimpleSelector,
isInPseudoClassContext,
isRootValid,
parseSelectorWithCache,
stringifySelector,
} from './helpers/selector';
Expand All @@ -46,9 +45,6 @@ const parseGlobal = SBTypesParsers[`-st-global`];
const parseExtends = SBTypesParsers[`-st-extends`];

export const processorWarnings = {
ROOT_AFTER_SPACING() {
return '".root" class cannot be used after native elements or selectors external to the stylesheet';
},
STATE_DEFINITION_IN_ELEMENT() {
return 'cannot define pseudo states inside a type selector';
},
Expand Down Expand Up @@ -376,10 +372,6 @@ export class StylableProcessor implements FeatureContext {
rule.selectorType = 'complex';
}

// ToDo: check cases of root in nested selectors?
if (!isRootValid(selectorAst)) {
this.diagnostics.warn(rule, processorWarnings.ROOT_AFTER_SPACING());
}
return locallyScoped;
}

Expand Down
64 changes: 2 additions & 62 deletions packages/core/test/diagnostics.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,7 @@ import {
expectTransformDiagnostics,
findTestLocations,
} from '@stylable/core-test-kit';
import {
processorWarnings,
transformerWarnings,
nativePseudoElements,
} from '@stylable/core';
import { processorWarnings, transformerWarnings, nativePseudoElements } from '@stylable/core';
import { CSSClass, CSSType } from '@stylable/core/dist/features';
import { generalDiagnostics } from '@stylable/core/dist/features/diagnostics';

Expand Down Expand Up @@ -284,59 +280,6 @@ describe('diagnostics: warnings and errors', () => {
});

describe('structure', () => {
describe('root', () => {
it('should return warning for ".root" after a selector', () => {
expectAnalyzeDiagnostics(
`
|.gaga .root|{}
`,
[{ message: processorWarnings.ROOT_AFTER_SPACING(), file: 'main.css' }]
);
});

it('should return warning for ".root" after global and local classes', () => {
expectAnalyzeDiagnostics(
`
|:global(*) .x .root|{}
`,
[{ message: processorWarnings.ROOT_AFTER_SPACING(), file: 'main.css' }]
);
});

it('should return warning for ".root" after a global and element', () => {
expectAnalyzeDiagnostics(
`
|:global(*) div .root|{}
`,
[
{
message: CSSType.diagnostics.UNSCOPED_TYPE_SELECTOR('div'),
file: 'main.css',
},
{ message: processorWarnings.ROOT_AFTER_SPACING(), file: 'main.css' },
]
);
});

it('should not return warning for ".root" after a global selector', () => {
expectAnalyzeDiagnostics(
`
:global(*) .root{}
`,
[]
);
});

it('should not return warning for ".root" after a complex global selector', () => {
expectAnalyzeDiagnostics(
`
:global(body[dir="rtl"] > header) .root {}
`,
[]
);
});
});

describe('-st-extends', () => {
it('should return warning when defined under complex selector', () => {
expectAnalyzeDiagnostics(
Expand Down Expand Up @@ -377,10 +320,7 @@ describe('diagnostics: warnings and errors', () => {
`,
[
{
message: processorWarnings.OVERRIDE_TYPED_RULE(
`-st-extends`,
'root'
),
message: processorWarnings.OVERRIDE_TYPED_RULE(`-st-extends`, 'root'),
file: 'main.css',
},
]
Expand Down
9 changes: 4 additions & 5 deletions packages/language-service/test/lib/diagnostics.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,17 @@ describe('diagnostics', () => {

const diagnostics = createDiagnostics(
{
[filePath]: '.gaga .root{}',
[filePath]: '.root:unknown{}',
},
filePath
);

expect(diagnostics).to.deep.include({
range: {
start: { line: 0, character: 0 },
start: { line: 0, character: 6 },
end: { line: 0, character: 13 },
},
message:
'".root" class cannot be used after native elements or selectors external to the stylesheet',
message: 'unknown pseudo-state "unknown"',
severity: 2,
source: 'stylable',
});
Expand All @@ -32,7 +31,7 @@ describe('diagnostics', () => {
it('should not duplicate diagnostics within multiple runs on the same file', () => {
const filePath = '/style.st.css';
const files = {
[filePath]: '.gaga .root{}',
[filePath]: '.root:unknown{}',
};
const fs = createMemoryFs(files);

Expand Down