Skip to content

Commit

Permalink
feat(compiler): attr to props for custom LWC elements (#102)
Browse files Browse the repository at this point in the history 
* Template compiler
* Fixing broken tests
* test failures
* First pass
* Using existing attribute to prop name map
* Cleanup
* Cleanup in class transform
* Removing error when defining getter that matches html global attribute
* Updaing observedAttributes validation
* Adding integration test
* Adding more validation
* Removing unnecessary prop and attribute error messages
* Fixing linting
* Fixing unit tests
* Removing data- attribtues from observed attributes callback
* observedAttributes error in compiler
* Adding proper prop name to ambigious warning
* Updating invalid attribute errors
* Pulling out attributeChangedCallback
* Updated error boundary tests
* new line
* Adding html getters
* tests
* Adding createAttributeSetter method
* Some test cleanup
* dir tests
* Wiring
* Getting defs working
* cleaning up some error messages for dom attributes/props
* Props
* reverting yarn lock
* feat(engine): add support for AOM properties on ShadowRoot (#119)
* Adding tests
* All tests passing
* Aria
* Aria
* Remove public prop check
* Adding ownPropertyDescriptor check for wire, track and public methods
* Updating compiler
* Removing unused type
* Fixing issue for ie11 id descriptor
* Using reduce to build default configs
* cleanup
* Adding back assertPublicAttributeColission
* Removing tabIndex'
* Fixing polyfill
* PR changes
* ponyfilling the AOM props in dom.ts
* consolidation of PR #102
* Remove attribute
* Adding accessibility tests
* AOM fixes in compiler. Integration tests
* Cleaning up hostAttrs
* Linting
* Remove extra hostAttrs=0 line
* Fix failing compiler test
* typos and error messages to be more explicits
* Adding minlength to props transform
* Adding some missing attributes
* removing accept-charset
  • Loading branch information
@davidturissini
davidturissini committed Mar 6, 2018
1 parent 0514227 commit 02a1320
Show file tree
Hide file tree
Showing 69 changed files with 2,415 additions and 715 deletions.
33 changes: 27 additions & 6 deletions packages/babel-plugin-transform-lwc-class/src/__tests__/api-decorator.spec.js
View file
Expand Up @@ -251,7 +251,7 @@ Text.publicMethods = ["m1"];`
}
`, {
error: {
message: 'test.js: Invalid property name is. "is" is a reserved attribute.',
message: 'test.js: Invalid property name "is". "is" is a reserved attribute.',
loc: {
line: 2,
column: 9
Expand Down Expand Up @@ -310,29 +310,50 @@ Test.publicProps = {
}
});

pluginTest('throws error if property name prefixed with "aria"', `
pluginTest('throws error if property name is ambigious', `
import { api } from 'engine';
export default class Test {
@api ariaDescribedby;
@api tabindex;
}
`, {
error: {
message: 'test.js: Invalid property name ariaDescribedby. Properties starting with "aria" are reserved attributes.',
message: 'test.js: Ambigious attribute name tabindex. tabindex will never be called from template because its corresponding property is camel cased. Consider renaming to "tabIndex".',
loc: {
line: 2,
column: 9
}
}
});

pluginTest('throws error if property name conflicts with global html attribute name', `
pluginTest('does not throw if property name prefixed with "aria"', `
import { api } from 'engine';
export default class Test {
@api ariaDescribedBy;
}
`, {
output: {
code: `export default class Test {
constructor() {
this.ariaDescribedBy = void 0;
}
}
Test.publicProps = {
ariaDescribedBy: {
config: 0
}
};`
}
});

pluginTest('throws error if property name conflicts with disallowed global html attribute name', `
import { api } from 'engine';
export default class Test {
@api slot;
}
`, {
error: {
message: 'test.js: Invalid property name slot. slot is a global HTML attribute, use attributeChangedCallback to observe this attribute.',
message: 'test.js: Invalid property name "slot". "slot" is a reserved attribute.',
loc: {
line: 2,
column: 9
Expand Down
18 changes: 18 additions & 0 deletions packages/babel-plugin-transform-lwc-class/src/__tests__/component.spec.js
View file
Expand Up @@ -61,6 +61,24 @@ Test.style = _tmpl.style;`
});
});

describe('observedAttributes array', () => {
pluginTest('throws if user defined observedAttributes', `
import { Element as Component } from 'engine';
export default class Test extends Component {
static observedAttributes = ['foo', 'title', 'tabindex'];
}
`, {
error: {
message: `test.js: Invalid static property "observedAttributes". "observedAttributes" cannot be used to track attribute changes. Define setters for "foo", "title", "tabIndex" instead.`,
loc: {
line: 1,
column: 7,
}
}
});
})

describe('render method', () => {
pluginTest('inject render method', `
import { Element } from "engine";
Expand Down
20 changes: 19 additions & 1 deletion packages/babel-plugin-transform-lwc-class/src/component.js
View file
@@ -1,7 +1,8 @@
const { basename } = require('path');
const moduleImports = require("@babel/helper-module-imports");
const { findClassMethod, staticClassProperty, getEngineImportSpecifiers, isComponentClass, isDefaultExport } = require('./utils');
const { LWC_PACKAGE_EXPORTS, LWC_COMPONENT_PROPERTIES } = require('./constants');
const { GLOBAL_ATTRIBUTE_MAP, LWC_PACKAGE_EXPORTS, LWC_COMPONENT_PROPERTIES } = require('./constants');
const CLASS_PROPERTY_OBSERVED_ATTRIBUTES = 'observedAttributes';

module.exports = function ({ types: t }) {
return {
Expand All @@ -15,6 +16,18 @@ module.exports = function ({ types: t }) {
path.get('local')
));
},
ClassProperty(path, state) {
if (isObservedAttributesStaticProperty(path)) {
const observedAttributeNames = path.node.value.elements.map((elem) => {
const { value } = elem;
const { propName = value } = (GLOBAL_ATTRIBUTE_MAP.get(value) || {});
return `"${propName}"`;
});
throw path.buildCodeFrameError(
`Invalid static property "observedAttributes". "observedAttributes" cannot be used to track attribute changes. Define setters for ${observedAttributeNames.join(', ')} instead.`
);
}
},
Class(path, state) {
const isComponent = isComponentClass(path, state.componentBaseClassImports);

Expand All @@ -37,6 +50,11 @@ module.exports = function ({ types: t }) {
}
};

function isObservedAttributesStaticProperty(classPropertyPath) {
const { static: isStaticProperty, key: { name: propertyName } } = classPropertyPath.node;
return (isStaticProperty && propertyName === CLASS_PROPERTY_OBSERVED_ATTRIBUTES);
}

function getBaseName({ file }) {
const classPath = file.opts.filename;
return basename(classPath, '.js');
Expand Down
48 changes: 44 additions & 4 deletions packages/babel-plugin-transform-lwc-class/src/constants.js
View file
@@ -1,6 +1,44 @@
const GLOBAL_ATTRIBUTE_SET = new Set([
'role', 'accesskey', 'class', 'contenteditable', 'contextmenu', 'dir', 'draggable', 'dropzone', 'hidden',
'id', 'itemprop', 'lang', 'slot', 'spellcheck', 'style', 'tabindex', 'title',
function globalAttributeObject(propName) {
return {
propName,
}
}

const GLOBAL_ATTRIBUTE_MAP = new Map([
['role', globalAttributeObject('role')],
['accesskey', globalAttributeObject('accessKey')],
['class', globalAttributeObject('class')],
['contenteditable', globalAttributeObject('contentEditable')],
['contextmenu', globalAttributeObject('contextmenu')],
['dir', globalAttributeObject('dir')],
['draggable', globalAttributeObject('draggable')],
['dropzone', globalAttributeObject('dropzone')],
['hidden', globalAttributeObject('hidden')],
['id', globalAttributeObject('id')],
['itemprop', globalAttributeObject('itemprop')],
['lang', globalAttributeObject('lang')],
['slot', globalAttributeObject('slot')],
['spellcheck', globalAttributeObject('spellcheck')],
['style', globalAttributeObject('style')],
['tabindex', globalAttributeObject('tabIndex')],
['title', globalAttributeObject('title')],
]);

// This set is for attributes that have a camel cased property name
// For example, div.tabIndex.
// We do not want users to define @api properties with these names
// Because the template will never call them. It'll alawys call the camel
// cased version.
const AMBIGIOUS_PROP_SET = new Set([
'bgcolor', 'accesskey', 'contenteditable', 'contextmenu', 'tabindex', 'maxlength', 'maxvalue'
]);


// This set is for attributes that can never be defined
// by users on their components.
// We throw for these.
const DISALLOWED_PROP_SET = new Set([
'is', 'class', 'slot', 'style'
]);

const LWC_PACKAGE_ALIAS = 'engine';
Expand Down Expand Up @@ -29,7 +67,9 @@ const DECORATOR_TYPES = {
}

module.exports = {
GLOBAL_ATTRIBUTE_SET,
AMBIGIOUS_PROP_SET,
DISALLOWED_PROP_SET,
GLOBAL_ATTRIBUTE_MAP,

LWC_PACKAGE_ALIAS,
LWC_PACKAGE_EXPORTS,
Expand Down
23 changes: 13 additions & 10 deletions packages/babel-plugin-transform-lwc-class/src/decorators/api/validate.js
View file
@@ -1,5 +1,11 @@
const { isApiDecorator } = require('./shared');
const { GLOBAL_ATTRIBUTE_SET, LWC_PACKAGE_EXPORTS: { TRACK_DECORATOR }, DECORATOR_TYPES } = require('../../constants');
const {
AMBIGIOUS_PROP_SET,
DISALLOWED_PROP_SET,
GLOBAL_ATTRIBUTE_MAP,
LWC_PACKAGE_EXPORTS: { TRACK_DECORATOR },
DECORATOR_TYPES
} = require('../../constants');

function validateConflict(path, decorators) {
const isPublicFieldTracked = decorators.some(decorator => (
Expand Down Expand Up @@ -30,11 +36,7 @@ function validatePropertyName(property) {

const propertyName = property.get('key.name').node;

if (propertyName === 'is') {
throw property.buildCodeFrameError(
`Invalid property name ${propertyName}. "is" is a reserved attribute.`
);
} else if (propertyName === 'part') {
if (propertyName === 'part') {
throw property.buildCodeFrameError(
`Invalid property name ${propertyName}. "part" is a future reserved attribute for web components.`
);
Expand All @@ -46,13 +48,14 @@ function validatePropertyName(property) {
throw property.buildCodeFrameError(
`Invalid property name ${propertyName}. Properties starting with "data" are reserved attributes.`
);
} else if (propertyName.startsWith('aria')) {
} else if (DISALLOWED_PROP_SET.has(propertyName)) {
throw property.buildCodeFrameError(
`Invalid property name ${propertyName}. Properties starting with "aria" are reserved attributes.`
`Invalid property name "${propertyName}". "${propertyName}" is a reserved attribute.`
);
} else if (GLOBAL_ATTRIBUTE_SET.has(propertyName)) {
} else if (AMBIGIOUS_PROP_SET.has(propertyName)) {
const { propName = propertyName } = GLOBAL_ATTRIBUTE_MAP.get(propertyName) || {};
throw property.buildCodeFrameError(
`Invalid property name ${propertyName}. ${propertyName} is a global HTML attribute, use attributeChangedCallback to observe this attribute.`
`Ambigious attribute name ${propertyName}. ${propertyName} will never be called from template because its corresponding property is camel cased. Consider renaming to "${propName}".`
);
}
}
Expand Down
3 changes: 2 additions & 1 deletion packages/lwc-engine/package.json
View file
Expand Up @@ -11,7 +11,8 @@
"build:all": "concurrently \"yarn build:es-and-cjs\" \"yarn build:umd:prod\" \"yarn build:umd:dev\"",
"build:umd:dev": "rollup -c scripts/rollup.config.umd.dev.js",
"build:umd:prod": "rollup -c scripts/rollup.config.umd.prod.js",
"build:es-and-cjs": "rollup -c scripts/rollup.config.es-and-cjs.js"
"build:es-and-cjs": "rollup -c scripts/rollup.config.es-and-cjs.js",
"test": "DIR=`pwd` && cd ../../ && jest $DIR"
},
"devDependencies": {
"concurrently": "^3.5.1",
Expand Down
87 changes: 0 additions & 87 deletions packages/lwc-engine/src/framework/__tests__/class-list.spec.ts
View file
Expand Up @@ -207,43 +207,6 @@ describe('class-list', () => {
});
});

it('should support adding new values to classList via attributeChangedCallback', () => {
const def = class MyComponent extends Element {
initClassNames() {
this.classList.add('classFromInit');
}

attributeChangedCallback(attributeName, oldValue, newValue) {
this.classList.add('classFromAttibuteChangedCb');
}
};
def.observedAttributes = ['title'];
def.publicMethods = ['initClassNames'];
const elm = createElement('x-foo', { is: def });
elm.initClassNames();
elm.setAttribute('title', 'title');
expect(elm.className).toBe('classFromInit classFromAttibuteChangedCb');
});

it('should support removing values from classList via attributeChangedCallback', () => {
const def = class MyComponent extends Element {
initClassNames() {
this.classList.add('theOnlyClassThatShouldRemain');
this.classList.add('classToRemoveDuringAttributeChangedCb');
}

attributeChangedCallback(attributeName, oldValue, newValue) {
this.classList.remove('classToRemoveDuringAttributeChangedCb');
}
};
def.observedAttributes = ['title'];
def.publicMethods = ['initClassNames'];
const elm = createElement('x-foo', { is: def });
elm.initClassNames();
elm.setAttribute('title', 'title');
expect(elm.className).toBe('theOnlyClassThatShouldRemain');
});

it('should support adding new values to classList via connectedCallback', () => {
const def = class MyComponent extends Element {
initClassNames() {
Expand Down Expand Up @@ -282,55 +245,5 @@ describe('class-list', () => {
expect(elm.className).toBe('theOnlyClassThatShouldRemain');
});
});

it('should support adding new values to classList via both attributeChangedCallback and classFromAttibuteChangedCb', () => {
const def = class MyComponent extends Element {
initClassNames() {
this.classList.add('classFromInit');
}

attributeChangedCallback(attributeName, oldValue, newValue) {
this.classList.add('classFromAttibuteChangedCb');
}

connectedCallback() {
this.classList.add('classFromConnectedCallback');
}
};
def.observedAttributes = ['title'];
def.publicMethods = ['initClassNames'];
const elm = createElement('x-foo', { is: def });
elm.initClassNames();
elm.setAttribute('title', 'title');
document.body.appendChild(elm);

expect(elm.className).toBe('classFromInit classFromAttibuteChangedCb classFromConnectedCallback');
});

it('should support removing values from classList via both attributeChangedCallback and classFromAttibuteChangedCb', () => {
const def = class MyComponent extends Element {
initClassNames() {
this.classList.add('theOnlyClassThatShouldRemain');
this.classList.add('classToRemoveDuringConnectedCb');
this.classList.add('classToRemoveDuringAttributeChangedCb');
}

attributeChangedCallback(attributeName, oldValue, newValue) {
this.classList.remove('classToRemoveDuringAttributeChangedCb');
}

connectedCallback() {
this.classList.remove('classToRemoveDuringConnectedCb');
}
};
def.observedAttributes = ['title'];
def.publicMethods = ['initClassNames'];
const elm = createElement('x-foo', { is: def });
elm.initClassNames();
elm.setAttribute('title', 'title');
document.body.appendChild(elm);

expect(elm.className).toBe('theOnlyClassThatShouldRemain');
});
});
});

0 comments on commit 02a1320

Please sign in to comment.