metal-web-component #261

robframpton · 2017-09-28T00:02:05Z

No description provided.

robframpton · 2017-09-28T01:01:26Z

I addressed the issue that was causing metal to be incompatible with the webcomponents polyfill, and now the tests are passing on all browsers that the polyfill supports.

However the tests are still failing on IE9/10. I tried conditionally loading the polyfill, but the issue persists. Any thoughts on how we can get around this?

It will also now work with JSX components, I've added tests to cover that use case.

jbalsas · 2017-09-28T10:56:55Z

Hey @Robert-Frampton! Nice seeing this almost there!!

What about having two different karma.config files, one for those needing the polyfill and one for those that don't support it and triggering two different tests? 🤷‍♂️

robframpton · 2017-09-28T19:51:21Z

@jbalsas the tests are passing now.

Originally we were extending HTMLElement with a class and using a babel-plugin-transform-custom-element-classes to get it to work, since extending that native class normally throws errors in certain browsers.

It turns out that the babel plugin was causing issues (the cause of the browserify error we were seeing). I removed the plugin, and just wrote the extension of HTMLElement without es2015 classes. The benefit is that we no longer have to have any special plugins to build it, so other devs could build this component from source if they wanted to without having to change their build tooling, and now it's more robust and doesn't break legacy browsers.

jbalsas · 2017-09-28T21:24:56Z

Awesome, @Robert-Frampton, great job!!

As part of closing this up... could we prepare a new tutorial in metaljs.com showing how to use this? We could maybe create a new branch where we push tutorials until we're ready to release a version with the updates.

What do you think?

robframpton · 2017-09-28T21:53:09Z

Hey @jbalsas,

Sounds good. Were you thinking of these kinds of tutorials? Or something more generic?

robframpton · 2017-09-28T23:59:51Z

I put together a quick guide on this branch. We could do something simple like this at first and provide a link to it in the metal-custom-element readme file, then later we can refactor some of these sections into actual tutorials like they have on wedeploy.

On another note, the naming is currently inconsistent in this package. The package name is metal-custom-element, but the method it exposes is called defineWebComponent. I'd like it to be consistent. Which one of these do you like more?

metal-custom-element / defineCustomElement
metal-web-component / defineWebComponent

jbalsas · 2017-09-29T07:07:53Z

Hey @Robert-Frampton, makes sense!

So, from "Web Components", we use the CustomElement and ShadowDOM (kinda) specs. We are not currently using neither Templates nor of course HTML imports.

I think I'd still rather go with the general metal-web-component / defineWebComponent approach, which would probably help us market it better... what do you think? Can you do the change here so we can merge it at once? 👼

The guide looks great, I think that is as much as we need right now 👍

robframpton · 2017-09-29T15:04:09Z

Hey @jbalsas, I agree. I think metal-web-component will make more sense to people (even if it's maybe less "accurate").

I'll make the changes now 👍

robframpton · 2017-09-29T15:41:14Z

bryceosterhaus/metaljs.com#69

robframpton · 2017-09-29T16:38:24Z

Hey @jbalsas, updated the package name and sent the PR for the guide on metaljs.com.

…be opted in on render

…al babel plugins

…empty object instead of null

…e component has rendered in connectedCallback. Make sure we don't reference this.component before it's defined.

robframpton · 2017-10-02T15:46:41Z

Hey @jbalsas, I think this is ready to merge whenever. Let me know if you see anything you think we should change.

jbalsas · 2017-10-10T12:00:32Z

packages/metal-dom/src/domNamed.js

@@ -168,7 +168,7 @@ export function append(parent, child) {
 	if (isString(child)) {
 		child = buildFragment(child);
 	}
-	if (child instanceof NodeList) {
+	if (isArrayLike(child)) {


Based on this change in implementation we have now a different behaviour:

const child = ['a', 'b', 'c']; dom.append(parent, child); // NOOP before -> 'abc' child after

Do we want to support this case? If so, we should probably add a test so it is clear. We might also simply do some duck-type-checking for NodeList... from the ones I've seen maybe one of these could be good enough:

const el = ??; const isNodeList = (typeof el.length != 'undefined' && typeof el.item != 'undefined'); const isNodeList = (typeof el.length == 'number' && typeof el.item == 'function' && typeof el.nextNode == 'function' && typeof el.reset == 'function');

jbalsas · 2017-10-10T12:01:44Z

packages/metal-web-component/package.json

+  "name": "metal-web-component",
+  "version": "1.0.0",
+  "description": "Web component (CustomElement) registration for Metal.js components.",
+  "license": "BSD-3-Clause",


We have a bit of a mix between BSD, BSD-3-Clause and MIT in this repo... might be good to have legal take a look :)

jbalsas · 2017-10-10T12:05:44Z

packages/metal-web-component/webcomponents_polyfill.js

+	script.src = '/base/packages/metal-web-component/node_modules/@webcomponents/webcomponentsjs/webcomponents-lite.js';
+	script.type = 'text/javascript';
+
+	document.body.appendChild(script);


Should we do something similar for the babel-polyfill, to make sure we don't load it when not necessary? I know it will probably noop in those cases, but it might have unintended side effects?

I tried conditionally loading this one as well, but it's harder to check if it's necessary than the webcomponents polyfill.

If you think it's necessary we can just explicitly check the user agent for every browser that needs it, but it won't be pretty.

jbalsas · 2017-10-10T12:06:24Z

packages/metal/src/coreNamed.js

+ * @param {?} val Variable to test.
+ * @return {boolean} Whether variable is an array/iterable.
+ */
+export function isArrayLike(val) {


See comment above... might be better to just check for isNodeListLike?

jbalsas · 2017-10-10T12:07:43Z

packages/metal-web-component/test/define_web_component.js

+	let el;
+
+	before(function() {
+		if (UA.matchUserAgent('MSIE') || isSafariVersion('8.0')) {


I assume we don't have something in UA to match version and user agent? :)

Correct, it would be nice to have something in UA that simply returns the major version number of the browser, regardless of what browser it is. We have a method in Portal that does something like that.

jbalsas · 2017-10-10T12:11:10Z

packages/metal-web-component/test/define_web_component.js

+		});
+	});
+
+	describe('Define JSX component', function() {


Why does this suite have different set of test cases than the Soy one? If they are the same, then maybe we should simply put them outside the soy one, or duplicate them in the jsx as well? Is there a chance one might work and not the other?

jbalsas · 2017-10-10T12:12:13Z

packages/metal-web-component/src/define_web_component.js

+	Object.setPrototypeOf(CustomElement, HTMLElement);
+
+	Object.assign(CustomElement.prototype, {
+		attributeChangedCallback: function(attrName, oldVal, newVal) {


Should we document what goes on in each callback, just for reference?

…-component

jbalsas · 2017-10-11T09:43:35Z

Hey @Robert-Frampton, LGTM!

I'm going to go ahead and merge to develop. My only doubt is what is the default browserlist value from the env preset. We need to support all latest releases (that includes IE11). Could you double check, just to be sure?

robframpton mentioned this pull request Sep 28, 2017

Remove webcomponent polyfill from tests and only run custom element tests on browsers with native support #258

Closed

robframpton force-pushed the webcomponents branch 3 times, most recently from d7cf48a to 127f2d4 Compare September 28, 2017 00:40

robframpton force-pushed the webcomponents branch 7 times, most recently from 9fad590 to 61f7d23 Compare September 28, 2017 19:34

Julien C and others added 9 commits September 29, 2017 10:31

Adds custom elements support for Metal components

bce50b5

Rename to match style guide

2dd0169

Update metal-custom-element dependencies

50c845c

Add useshadowdom attribute to custom elements so that shadow DOM can …

09b23e7

…be opted in on render

Generate package-lock.json for metal-custom-element package

b6fd2dc

Skip metal-custom-element tests when IE10 and below

d997bcd

Add isArrayLike helper method for checking if value is iterable

c6e0455

Custom elements should work with JSX components

3a4f12c

Update version of Edge and add latest version of Safari

2da87f3

Robert-Frampton added 11 commits September 29, 2017 10:31

Skip test for Safari 8

c7576be

Dynamically load webcomponents polyfill as it's incompatible with IE9/10

22e309f

Remove additional babel plugins that were added for custom elements

3286db0

Create CustomComponent in such a way that it doesn't require addition…

8679466

…al babel plugins

Regen package-lock.json

2d4b77c

Regen metal-custom-element/package-lock.json

bd132b4

Clearer formatting

d7db927

Rename (no changes)

025ca4b

Change all references of metal-custom-element to metal-web-component

496ce68

Regen metal-web-component/package-lock.json

3fac83e

After updating the metal dependency getStaticProperty nows returns …

c559dde

…empty object instead of null

robframpton force-pushed the webcomponents branch from bcfd972 to c559dde Compare September 29, 2017 17:39

Robert-Frampton added 3 commits September 29, 2017 11:09

Regen packag-lock.json (ensuring it's correct after rebase)

6d306ed

attributeChangedCallback is invoked for inital attribute values befor…

3cd95c4

…e component has rendered in connectedCallback. Make sure we don't reference this.component before it's defined.

Deserialize attributes that are json before passing to metal component

78f1a06

jbalsas mentioned this pull request Oct 3, 2017

Automate creation of CustomElements #268

Closed

3 tasks

robframpton changed the title ~~Try loading polyfill from test~~ metal-web-component Oct 9, 2017

jbalsas suggested changes Oct 10, 2017

View reviewed changes

robframpton changed the base branch from master to develop October 10, 2017 16:23

Robert-Frampton added 4 commits October 10, 2017 11:28

Replace isArrayLike with isNodeListLike

d48ee9d

Add JSDocs for CustomElement

84927ea

Test JSX components to the same degree as Soy components in metal-web…

c7c045a

…-component

Replace babel-preset-es2015 with babel-preset-env

fafd282

robframpton force-pushed the webcomponents branch from 226770e to 116f523 Compare October 10, 2017 22:30

Regen metal-web-component/package-lock.json

b71e390

robframpton force-pushed the webcomponents branch from 116f523 to b71e390 Compare October 10, 2017 23:10

jbalsas added this to the 2.14.0 milestone Oct 11, 2017

jbalsas merged commit 83a264c into metal:develop Oct 11, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

metal-web-component #261

metal-web-component #261

robframpton commented Sep 28, 2017

robframpton commented Sep 28, 2017 •

edited

Loading

jbalsas commented Sep 28, 2017

robframpton commented Sep 28, 2017

jbalsas commented Sep 28, 2017

robframpton commented Sep 28, 2017 •

edited

Loading

robframpton commented Sep 28, 2017

jbalsas commented Sep 29, 2017

robframpton commented Sep 29, 2017

robframpton commented Sep 29, 2017

robframpton commented Sep 29, 2017

robframpton commented Oct 2, 2017

jbalsas Oct 10, 2017

jbalsas Oct 10, 2017

jbalsas Oct 10, 2017

robframpton Oct 10, 2017

jbalsas Oct 10, 2017

jbalsas Oct 10, 2017

robframpton Oct 10, 2017

jbalsas Oct 10, 2017

jbalsas Oct 10, 2017

jbalsas commented Oct 11, 2017

metal-web-component #261

metal-web-component #261

Conversation

robframpton commented Sep 28, 2017

robframpton commented Sep 28, 2017 • edited Loading

jbalsas commented Sep 28, 2017

robframpton commented Sep 28, 2017

jbalsas commented Sep 28, 2017

robframpton commented Sep 28, 2017 • edited Loading

robframpton commented Sep 28, 2017

jbalsas commented Sep 29, 2017

robframpton commented Sep 29, 2017

robframpton commented Sep 29, 2017

robframpton commented Sep 29, 2017

robframpton commented Oct 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbalsas commented Oct 11, 2017

robframpton commented Sep 28, 2017 •

edited

Loading

robframpton commented Sep 28, 2017 •

edited

Loading