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: open usage of data-* and aria-* attributes in CSS #632

Merged
merged 4 commits into from
Sep 12, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
18 changes: 11 additions & 7 deletions packages/postcss-plugin-lwc/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -127,16 +127,20 @@ div {

Since LWC uses the HTML attribute syntax to define properties on components, it will be misleading to use attribute selectors when styling a component. For this reason the CSS transform restricts the usage of CSS attribute selectors.

* CSS selectors using [Global HTML attributes](https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes) are allowed.
* Usage of attributes are only allowed in compound selectors with known tag selectors
* CSS selectors using [Global HTML attributes](https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes), [data-* attributes](https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/data-*) and [aria-* attributes](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA) are allowed.
* Usage of attributes are only allowed in compound selectors with known tag selectors.

```css
[hidden] {} /* ✅ OK - global HTML attribute selector */
x-btn[hidden] {} /* ✅ OK - global HTML attribute selector */
[hidden] {} /* ✅ OK - global HTML attribute selector */
x-btn[hidden] {} /* ✅ OK - global HTML attribute selector */

[min=0] {} /* 🚨 ERROR - the compound selector is not specific enough */
input[min=0] {} /* ✅ OK - "min" attribute is a known special attribute on the "input" element */
x-btn[min=0] {} /* 🚨 ERROR - invalid usage "min" attribute on "x-btn" */
[data-foo] {} /* ✅ OK - data-* attribute selector */

[aria-hidden="true"] {} /* ✅ OK - aria-* attribute selector */

[min=0] {} /* 🚨 ERROR - the compound selector is not specific enough */
input[min=0] {} /* ✅ OK - "min" attribute is a known special attribute on the "input" element */
x-btn[min=0] {} /* 🚨 ERROR - invalid usage "min" attribute on "x-btn" */
```

## Selector scoping caveats
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,4 +107,33 @@ describe('attribute validation', () => {
column: 2,
});
});

it('should allow usage of data-* attributes', async () => {
await expect(process('[data-foo] {}')).resolves.toBeDefined();
await expect(process('[data-foo="bar"] {}')).resolves.toBeDefined();
});

it('should forbid usage of the data attribute', async () => {
await expect(process('[data] {}')).rejects.toMatchObject({
message: expect.stringMatching(
/is too generic/,
),
file: FILE_NAME,
line: 1,
column: 2,
});
await expect(process('div[data] {}')).rejects.toMatchObject({
message: expect.stringMatching(
/"\[data\]" can't be applied to match on <div>/,
),
file: FILE_NAME,
line: 1,
column: 4,
});
});

it('should allow usage of ARIA attributes', async () => {
await expect(process('[data-foo] {}')).resolves.toBeDefined();
await expect(process('[data-foo="bar"] {}')).resolves.toBeDefined();
});
});
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
export function isValidAttribute(tagName: string, attribute: string): boolean {
// All the element possess global HTML attributes.
if (GLOBAL_ATTRIBUTE_SET.has(attribute)) {
return true;
}
export function isGlobalAttribute(attributeName: string): boolean {
return GLOBAL_ATTRIBUTE_SET.has(attributeName);
}

export function isAriaAttribute(attributeName: string): boolean {
return attributeName.startsWith('aria-*');
}

export function isDataAttribute(attributeName: string): boolean {
return attributeName.startsWith('data-');
}

export function isKnowAttributeOnElement(tagName: string, attributeName: string): boolean {
// We can't validate the attribute on custom elements.
const isCustomElement = tagName.includes('-');
if (isCustomElement) {
Expand All @@ -12,12 +19,12 @@ export function isValidAttribute(tagName: string, attribute: string): boolean {

// Finally check in the list of known attributes for standard elements.
return (
Array.isArray(HTML_ATTRIBUTES_REVERSE_LOOKUP[attribute]) &&
HTML_ATTRIBUTES_REVERSE_LOOKUP[attribute].includes(tagName)
Array.isArray(HTML_ATTRIBUTES_REVERSE_LOOKUP[attributeName]) &&
HTML_ATTRIBUTES_REVERSE_LOOKUP[attributeName].includes(tagName)
);
}

export const GLOBAL_ATTRIBUTE_SET: Set<string> = new Set([
const GLOBAL_ATTRIBUTE_SET: Set<string> = new Set([
'role',
'accesskey',
'class',
Expand All @@ -37,7 +44,7 @@ export const GLOBAL_ATTRIBUTE_SET: Set<string> = new Set([
'title',
]);

export const HTML_ATTRIBUTES_REVERSE_LOOKUP: { [attr: string]: string[] } = {
const HTML_ATTRIBUTES_REVERSE_LOOKUP: { [attr: string]: string[] } = {
'xlink:href': [
'use',
],
Expand Down
53 changes: 29 additions & 24 deletions packages/postcss-plugin-lwc/src/selector-scoping/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@ import {
} from 'postcss-selector-parser';

import {
GLOBAL_ATTRIBUTE_SET,
isValidAttribute,
} from './html-attributes';
isGlobalAttribute,
isAriaAttribute,
isDataAttribute,
isKnowAttributeOnElement,
} from '../helpers/html-attributes';

const DEPRECATED_SELECTORS = new Set(['/deep/', '::shadow', '>>>']);
const UNSUPPORTED_SELECTORS = new Set(['::slotted', ':root', ':host-context']);
Expand Down Expand Up @@ -47,18 +49,17 @@ function validateSelectors(root: Root) {
function validateAttribute(root: Root) {
root.walk(node => {
if (isAttribute(node)) {
const { attribute, sourceIndex } = node;
const { attribute: attributeName, sourceIndex } = node;

// Global HTML attributes are valid on all the element, custom or not.
const isGlobalHTMLAttribute = GLOBAL_ATTRIBUTE_SET.has(attribute);
if (isGlobalHTMLAttribute) {
// Let's check if the attribute name is either a Global HTML attribute, an ARIA attribute
// or a data-* attribute since those are available on all the elements.
if (isGlobalAttribute(attributeName) || isAriaAttribute(attributeName) || isDataAttribute(attributeName)) {
return;
}

// If the attribute is not a global one we need to validate it's usage. Walking
// backward the selector chain to find a tag selector in the compound selector.
// The lookup stop when either a tag is found or when reaching the next
// combinator - which indicates the end of the compound selector.
// If the attribute name is not a globally available attribute, the attribute selector is required
// to be associated with a tag selector, so we can validate it's usage. Let's walk the compound selector
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: it's => its

// backward to find the associated tag selector.
let tagSelector: Tag | undefined = undefined;
let runner = node.prev();

Expand All @@ -74,31 +75,35 @@ function validateAttribute(root: Root) {
}
}

// Error out when not tag selector is present in the compound selector.
if (!tagSelector) {
// If the tag selector is not present in the compound selector, we need to warn the user that
// the compound selector need to be more specific.
if (tagSelector === undefined) {
const message = [
`Attribute selector "${node}" is too generic. `,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Add a tag selector to validate the usage of the attribute selector" doesn't sound quite right because it sounds vague without the right context. What do you think about:

For validation purposes, attributes that are not global attributes must be associated with a tag name when used in a CSS selector (e.g., "input[min]" instead of "[min]").

`Add a tag selector to validate the usage of the attribute selector.`,
];

throw root.error(
`Selector "${node}" is too generic, add a tag selector.`,
message.join(''),
{
index: sourceIndex,
word: attribute,
},
word: attributeName,
}
);
}

// Check if the attribute is permitted for the considered tag.
const isValidSelector = isValidAttribute(
tagSelector.value,
attribute,
);
if (!isValidSelector) {
// If compound selector is associated with a tag selector, we can validate the usage of the
// attribute against the specific tag.
const { value: tagName } = tagSelector;
if (!isKnowAttributeOnElement(tagName, attributeName)) {
const message = [
`Attribute selector "${node}" can't be applied to match on <${tagSelector}>. `,
`Attribute selector "${node}" can't be applied to match on <${tagSelector.value}>. `,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about something like:

"${tagName}${node}" is an invalid selector because "${attributeName}" is not a known attribute of "${tagName}".

`Use another method to match on the element.`,
];

throw root.error(message.join(''), {
index: sourceIndex,
word: attribute,
word: attributeName,
});
}
}
Expand Down