fix(engine): exposing host #705

davidturissini · 2018-10-03T22:53:00Z

Details

Exposing host on shadowRoot

Does this PR introduce a breaking change?

Yes
No

salesforce-best-lwc-internal · 2018-10-03T22:58:14Z

Benchmark results

Base commit: 91bc49b | Target commit: a3738b2

ravijayaramappa · 2018-10-03T23:41:44Z

packages/lwc-engine/src/framework/__tests__/restrictions.spec.ts

@@ -24,5 +24,18 @@ describe('restrictions', () => {
                template.querySelector('p').childNodes;
            }).toLogWarning('Discouraged access to property \'childNodes\' on \'Node\': It returns a live NodeList and should not be relied upon. Instead, use \'querySelectorAll\' which returns a static NodeList.');
        });
+
+        it('should not restrict host', () => {


This test is redundant. It is a subset of the other test you are adding above.

That is true, but I couldn't decide if I should test the restriction removal, or the host behavior. So I did both

Input on which I should keep @ravijayaramappa ?

😄 First world problems.
IMO, you should keep the test in shadow-root.spec.ts coz it is now part of the shadow-root spec.

Agree with Ravi. Makes more sense to test new functionality closer to the implementation rather than add a test here for what we used to restrict.

diervo

Braze yourself!

ravijayaramappa · 2018-10-05T18:32:54Z

packages/lwc-engine/src/polyfills/shadow-root/detect.ts

@@ -0,0 +1,3 @@
+export default function detect(): boolean {
+    return true;


typeof (window as any).ShadowRoot !== "undefined";? Or this is by choice?

I'm still working on the detection logic. Unfortunately, in Selenium, we always want to patch window.ShadowDom because internally Selenium uses instanceof to verify that a value is Shadow

pmdartus · 2018-10-09T20:07:57Z

packages/lwc-integration/scripts/wdio.sauce.conf.js

@@ -49,7 +49,8 @@ const compatBrowsers = [
        commonName: 'ie11',
        browserName: 'internet explorer',
        platform: 'Windows 10',
-        version: '11.103'
+        version: '11.103',
+        iedriverVersion: '3.4.0'


Adding a small comment here would be interesting, explaining why the previous version are incompatible.

By the way did we tests against the other browsers?

What do you mean "test against other browsers?" Our CI is running against everything...

pmdartus · 2018-10-09T20:11:19Z

packages/lwc-engine/src/polyfills/shadow-root/detect.ts

@@ -0,0 +1,3 @@
+export default function detect(): boolean {


We should add a README.md explaining this polyfill

pmdartus · 2018-10-09T20:12:11Z

packages/lwc-engine/src/polyfills/document-element-from-point/detect.ts

@@ -0,0 +1,3 @@
+export default function detect(): boolean {


We should add a README.md explaining this polyfill.

pmdartus · 2018-10-09T20:14:23Z

packages/lwc-engine/src/faux-shadow/shadow-root.ts

-    }) as ShadowRoot;
+
+    // These cannot be patched when module is loaded because
+    // Element.prototype needs to be patched first, which happens


Looks like another circular dependency issue.

salesforce-best-lwc-internal · 2018-10-09T21:35:40Z

Benchmark results

Base commit: 7a22a8b | Target commit: 1221212

caridy · 2018-10-10T01:22:22Z

packages/lwc-engine/src/faux-shadow/shadow-root.ts

+        removeShadowRootEventListener(this, type, listener, options);
+    }
+    compareDocumentPosition(otherNode: Node | SyntheticShadowRoot) {
+        // this API might be called with proxies


proxies? what proxies? lol

I think all those comments about proxies can be removed.

caridy · 2018-10-10T01:29:32Z

packages/lwc-engine/src/faux-shadow/shadow-root.ts

+        const elements = elementsFromPoint.call(document, left, top);
+        const hostKey = getNodeKey(this.host);
+        let topElement = null;
+        for (let i = elements.length - 1; i >= 0; i -= 1) {


this is very redundant, I think calling getFirstMatch(host, elements) might be sufficient here, that already take care of the undefined keys, etc.

caridy · 2018-10-10T01:31:39Z

packages/lwc-engine/src/polyfills/document-element-from-point/polyfill.ts

+        for (let i = length - 1; i >= 0; i -= 1) {
+            const el = elements[i];
+            const ownerKey = getNodeOwnerKey(el);
+            if (isUndefined(ownerKey)) {


I think this logic does not cover manually inserted elements inside LWC, but I think that's a very edge case for now.

caridy · 2018-10-10T01:33:02Z

packages/lwc-engine/src/polyfills/shadow-root/polyfill.ts

+import { SyntheticShadowRoot } from "../../faux-shadow/shadow-root";
+
+export default function apply() {
+    (window as any).ShadowRoot = SyntheticShadowRoot;


I'm fine with this, but I think this constructor should not be callable/newable, so it matches the native one. In this PR, you'are making that constructor a valid target for new.

* feat(engine): dom patching * fix(engine): cleanup descriptors * feat(engine): rebase * fix(engine): linting * fix(engine): pr feedback * fix(engine): lint * fix(compiler): updated snapshot testing to account for shadow dom * fix(engine): pr feedback * fix(engine): fixing restrictions on custom elements * fix(engine): exposing host (#705) * fix(engine): exposing host * fix(engine): shadow root * wip(engine): linting and type errors * fix(engine): cleaning up shadow root detection * fix(engine): removing restrictions test * fix(engine): fixing element from point methods * fix(engine): disabling some attribute integration test for now * fix(engine): linting * fix(engine): upgrade ie11 driver * fix(engine): compat test fix * fix(engine): applying elementFromPoint on document * fix(engine): linting * fix(engine): reverting unnecessary changes * fix(engine): removing skip in test * fix(engine): polyfill readmes * fix(engine): review feedback * fix(engine): integration tests for elementFromPoint * fix(engine): ie11 elementsFromPoint * fix(engine): feedback * fix(engine): ts fix

* refactor(engine): removal of is attr and forceTagName refactor(engine): removal of is attr and forceTagName fix(engine): removing integration test for forceTagName refactor(engine): snabbdom lite - phase 1 refactor(engine): adding hooks.ts refactor(engine): removing global hooks refactor(engine): PR 606 refactor(engine): splitting style and class modules into static and dynamic test(engine): ready for the final push refactor(engine): making hooks mandatory. refactor(engine): making all hooks functions refactor(engine): dynamic diff vs static diff refactor(engine): using two diff algos refactor(engine): removing htmlapi in favor of pure patching fix(engine): missing argument when determining diff algo fix(engine): adding tests for dynamic diff fix(engine): clean up fix(engine): integration test failures * refactor(engine): proto chaining * fix(engine): wrong auto import * fix(engine): correcting the proto chain * refactor(engine): implementing the base element proto chain * test(engine): adding more tests for restrictions on elements * refactor(engine): rename to BaseBridgeElement * feat(engine): dom patching (#688) * feat(engine): dom patching * fix(engine): cleanup descriptors * feat(engine): rebase * fix(engine): linting * fix(engine): pr feedback * fix(engine): lint * fix(compiler): updated snapshot testing to account for shadow dom * fix(engine): pr feedback * fix(engine): fixing restrictions on custom elements * fix(engine): exposing host (#705) * fix(engine): exposing host * fix(engine): shadow root * wip(engine): linting and type errors * fix(engine): cleaning up shadow root detection * fix(engine): removing restrictions test * fix(engine): fixing element from point methods * fix(engine): disabling some attribute integration test for now * fix(engine): linting * fix(engine): upgrade ie11 driver * fix(engine): compat test fix * fix(engine): applying elementFromPoint on document * fix(engine): linting * fix(engine): reverting unnecessary changes * fix(engine): removing skip in test * fix(engine): polyfill readmes * fix(engine): review feedback * fix(engine): integration tests for elementFromPoint * fix(engine): ie11 elementsFromPoint * fix(engine): feedback * fix(engine): ts fix * wip: yarn update * fix(engine): fixing snapshot test

refactor(engine): removal of is attr and forceTagName fix(engine): removing integration test for forceTagName fix: remove custom element transformation from CSS (#695) fix(compiler): fixing fixtures refactor(engine): snabbdom lite - phase 1 (#606) * refactor(engine): removal of is attr and forceTagName refactor(engine): removal of is attr and forceTagName fix(engine): removing integration test for forceTagName refactor(engine): snabbdom lite - phase 1 refactor(engine): adding hooks.ts refactor(engine): removing global hooks refactor(engine): PR 606 refactor(engine): splitting style and class modules into static and dynamic test(engine): ready for the final push refactor(engine): making hooks mandatory. refactor(engine): making all hooks functions refactor(engine): dynamic diff vs static diff refactor(engine): using two diff algos refactor(engine): removing htmlapi in favor of pure patching fix(engine): missing argument when determining diff algo fix(engine): adding tests for dynamic diff fix(engine): clean up fix(engine): integration test failures * refactor(engine): proto chaining * fix(engine): wrong auto import * fix(engine): correcting the proto chain * refactor(engine): implementing the base element proto chain * test(engine): adding more tests for restrictions on elements * refactor(engine): rename to BaseBridgeElement * feat(engine): dom patching (#688) * feat(engine): dom patching * fix(engine): cleanup descriptors * feat(engine): rebase * fix(engine): linting * fix(engine): pr feedback * fix(engine): lint * fix(compiler): updated snapshot testing to account for shadow dom * fix(engine): pr feedback * fix(engine): fixing restrictions on custom elements * fix(engine): exposing host (#705) * fix(engine): exposing host * fix(engine): shadow root * wip(engine): linting and type errors * fix(engine): cleaning up shadow root detection * fix(engine): removing restrictions test * fix(engine): fixing element from point methods * fix(engine): disabling some attribute integration test for now * fix(engine): linting * fix(engine): upgrade ie11 driver * fix(engine): compat test fix * fix(engine): applying elementFromPoint on document * fix(engine): linting * fix(engine): reverting unnecessary changes * fix(engine): removing skip in test * fix(engine): polyfill readmes * fix(engine): review feedback * fix(engine): integration tests for elementFromPoint * fix(engine): ie11 elementsFromPoint * fix(engine): feedback * fix(engine): ts fix * wip: yarn update * fix(engine): fixing snapshot test

* refactor(engine): removal of is attr and forceTagName * fix(engine): removing integration test for forceTagName * fix: remove custom element transformation from CSS (#695) * fix(compiler): fixing fixtures * refactor(engine): snabbdom lite - phase 1 (#606) * refactor(engine): removal of is attr and forceTagName * refactor(engine): removal of is attr and forceTagName * fix(engine): removing integration test for forceTagName * refactor(engine): snabbdom lite - phase 1 * refactor(engine): adding hooks.ts * refactor(engine): removing global hooks * refactor(engine): splitting style and class modules into static and dynamic * test(engine): ready for the final push * refactor(engine): making hooks mandatory. * refactor(engine): making all hooks functions * refactor(engine): dynamic diff vs static diff * refactor(engine): using two diff algos * refactor(engine): removing htmlapi in favor of pure patching * fix(engine): missing argument when determining diff algo * fix(engine): adding tests for dynamic diff * fix(engine): clean up * fix(engine): integration test failures * refactor(engine): proto chaining * fix(engine): wrong auto import * fix(engine): correcting the proto chain * refactor(engine): implementing the base element proto chain * test(engine): adding more tests for restrictions on elements * refactor(engine): rename to BaseBridgeElement * feat(engine): dom patching (#688) * feat(engine): dom patching * fix(engine): cleanup descriptors * feat(engine): rebase * fix(engine): linting * fix(engine): pr feedback * fix(engine): lint * fix(compiler): updated snapshot testing to account for shadow dom * fix(engine): pr feedback * fix(engine): fixing restrictions on custom elements * fix(engine): exposing host (#705) * fix(engine): exposing host * fix(engine): shadow root * wip(engine): linting and type errors * fix(engine): cleaning up shadow root detection * fix(engine): removing restrictions test * fix(engine): fixing element from point methods * fix(engine): disabling some attribute integration test for now * fix(engine): linting * fix(engine): upgrade ie11 driver * fix(engine): compat test fix * fix(engine): applying elementFromPoint on document * fix(engine): linting * fix(engine): reverting unnecessary changes * fix(engine): removing skip in test * fix(engine): polyfill readmes * fix(engine): review feedback * fix(engine): integration tests for elementFromPoint * fix(engine): ie11 elementsFromPoint * fix(engine): feedback * fix(engine): ts fix * wip: yarn update * fix(engine): fixing snapshot test wip: styles still broken wip: fix styles

* * refactor(engine): removal of is attr and forceTagName * refactor(engine): removal of is attr and forceTagName * fix(engine): removing integration test for forceTagName * fix: remove custom element transformation from CSS (#695) * fix(compiler): fixing fixtures * refactor(engine): snabbdom lite - phase 1 (#606) * refactor(engine): removal of is attr and forceTagName * refactor(engine): removal of is attr and forceTagName * fix(engine): removing integration test for forceTagName * refactor(engine): snabbdom lite - phase 1 * refactor(engine): adding hooks.ts * refactor(engine): removing global hooks * refactor(engine): splitting style and class modules into static and dynamic * test(engine): ready for the final push * refactor(engine): making hooks mandatory. * refactor(engine): making all hooks functions * refactor(engine): dynamic diff vs static diff * refactor(engine): using two diff algos * refactor(engine): removing htmlapi in favor of pure patching * fix(engine): missing argument when determining diff algo * fix(engine): adding tests for dynamic diff * fix(engine): clean up * fix(engine): integration test failures * refactor(engine): proto chaining * fix(engine): wrong auto import * fix(engine): correcting the proto chain * refactor(engine): implementing the base element proto chain * test(engine): adding more tests for restrictions on elements * refactor(engine): rename to BaseBridgeElement * feat(engine): dom patching (#688) * feat(engine): dom patching * fix(engine): cleanup descriptors * feat(engine): rebase * fix(engine): linting * fix(engine): pr feedback * fix(engine): lint * fix(compiler): updated snapshot testing to account for shadow dom * fix(engine): pr feedback * fix(engine): fixing restrictions on custom elements * fix(engine): exposing host (#705) * fix(engine): exposing host * fix(engine): shadow root * wip(engine): linting and type errors * fix(engine): cleaning up shadow root detection * fix(engine): removing restrictions test * fix(engine): fixing element from point methods * fix(engine): disabling some attribute integration test for now * fix(engine): linting * fix(engine): upgrade ie11 driver * fix(engine): compat test fix * fix(engine): applying elementFromPoint on document * fix(engine): linting * fix(engine): reverting unnecessary changes * fix(engine): removing skip in test * fix(engine): polyfill readmes * fix(engine): review feedback * fix(engine): integration tests for elementFromPoint * fix(engine): ie11 elementsFromPoint * fix(engine): feedback * fix(engine): ts fix * wip: yarn update * fix(engine): fixing snapshot test wip: styles still broken wip: fix styles * refactor(engine): preliminar work for tabindex * feat(engine): prototype over descriptors * refactor(engine): preliminar work for focus and blur patch * fix(engine): invalid import statement * refactor(engine): preliminar work for focus and blur patch * fix(engine): make shadowroot a fragment * fix(engine): make shadowroot a fragment * fix(engine): make shadowroot a fragment * fix(engine): make shadowroot a fragment * feat(engine): active element for shadow roots (#740) * feat(engine): active element for shadow roots * fix(engine): linting * fix(docs): updated readme * fix(engine): types * fix(engine): nodelist iterator * fix(engine): nodelist iterator * chore(engine): rebasing next-rebased * refactor(engine): implement delegatesFocus flag * test(engine): implement delegatesFocus flag tests * chore(engine): fixing dval mess with the history * fix: newlines on tests * feat: add some polyfill tests * fix(engine): tests * fix(engine): review for PR 747

* refactor(engine): removal of is attr and forceTagName * fix(engine): removing integration test for forceTagName * fix: remove custom element transformation from CSS (#695) * fix(compiler): fixing fixtures * refactor(engine): snabbdom lite - phase 1 (#606) * refactor(engine): removal of is attr and forceTagName * refactor(engine): removal of is attr and forceTagName * fix(engine): removing integration test for forceTagName * refactor(engine): snabbdom lite - phase 1 * refactor(engine): adding hooks.ts * refactor(engine): removing global hooks * refactor(engine): splitting style and class modules into static and dynamic * test(engine): ready for the final push * refactor(engine): making hooks mandatory. * refactor(engine): making all hooks functions * refactor(engine): dynamic diff vs static diff * refactor(engine): using two diff algos * refactor(engine): removing htmlapi in favor of pure patching * fix(engine): missing argument when determining diff algo * fix(engine): adding tests for dynamic diff * fix(engine): clean up * fix(engine): integration test failures * refactor(engine): proto chaining * fix(engine): wrong auto import * fix(engine): correcting the proto chain * refactor(engine): implementing the base element proto chain * test(engine): adding more tests for restrictions on elements * refactor(engine): rename to BaseBridgeElement * feat(engine): dom patching (#688) * feat(engine): dom patching * fix(engine): cleanup descriptors * feat(engine): rebase * fix(engine): linting * fix(engine): pr feedback * fix(engine): lint * fix(compiler): updated snapshot testing to account for shadow dom * fix(engine): pr feedback * fix(engine): fixing restrictions on custom elements * fix(engine): exposing host (#705) * fix(engine): exposing host * fix(engine): shadow root * wip(engine): linting and type errors * fix(engine): cleaning up shadow root detection * fix(engine): removing restrictions test * fix(engine): fixing element from point methods * fix(engine): disabling some attribute integration test for now * fix(engine): linting * fix(engine): upgrade ie11 driver * fix(engine): compat test fix * fix(engine): applying elementFromPoint on document * fix(engine): linting * fix(engine): reverting unnecessary changes * fix(engine): removing skip in test * fix(engine): polyfill readmes * fix(engine): review feedback * fix(engine): integration tests for elementFromPoint * fix(engine): ie11 elementsFromPoint * fix(engine): feedback * fix(engine): ts fix * wip: yarn update * fix(engine): fixing snapshot test

fix(engine): exposing host

a3738b2

caridy approved these changes Oct 3, 2018

View reviewed changes

ravijayaramappa reviewed Oct 3, 2018

View reviewed changes

diervo approved these changes Oct 4, 2018

View reviewed changes

fix(engine): shadow root

9dbcf4c

ravijayaramappa reviewed Oct 5, 2018

View reviewed changes

davidturissini added 6 commits October 5, 2018 11:40

wip(engine): linting and type errors

e656a77

fix(engine): cleaning up shadow root detection

e605dfd

fix(engine): removing restrictions test

ab8c820

fix(engine): fixing element from point methods

0ac93a1

fix(engine): disabling some attribute integration test for now

03d2251

fix(engine): linting

376876f

salesforce deleted a comment from salesforce-best-lwc-internal bot Oct 5, 2018

davidturissini added 6 commits October 8, 2018 16:31

fix(engine): upgrade ie11 driver

dd2eaa6

fix(engine): compat test fix

0adacea

fix(engine): applying elementFromPoint on document

1e6f3c5

fix(engine): linting

831b884

fix(engine): reverting unnecessary changes

0f407d0

fix(engine): removing skip in test

7bf98d7

salesforce deleted a comment from salesforce-best-lwc-internal bot Oct 9, 2018

pmdartus reviewed Oct 9, 2018

View reviewed changes

fix(engine): polyfill readmes

1221212

davidturissini merged commit 047a5b8 into dturissini/dom-prototype-inserting Oct 9, 2018

davidturissini deleted the dturissini/shadow-host branch October 9, 2018 22:24

caridy reviewed Oct 10, 2018

View reviewed changes

diervo mentioned this pull request Oct 13, 2018

feat: next rebased #734

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(engine): exposing host #705

fix(engine): exposing host #705

davidturissini commented Oct 3, 2018

salesforce-best-lwc-internal bot commented Oct 3, 2018

ravijayaramappa Oct 3, 2018

davidturissini Oct 3, 2018

davidturissini Oct 3, 2018

ravijayaramappa Oct 3, 2018

trevor-bliss Oct 4, 2018

diervo left a comment

ravijayaramappa Oct 5, 2018

davidturissini Oct 5, 2018

pmdartus Oct 9, 2018

pmdartus Oct 9, 2018

davidturissini Oct 9, 2018

pmdartus Oct 9, 2018

pmdartus Oct 9, 2018

pmdartus Oct 9, 2018

salesforce-best-lwc-internal bot commented Oct 9, 2018

caridy Oct 10, 2018

caridy Oct 10, 2018

caridy Oct 10, 2018

caridy Oct 10, 2018

caridy Oct 10, 2018

		@@ -0,0 +1,3 @@
		export default function detect(): boolean {
		return true;

fix(engine): exposing host #705

fix(engine): exposing host #705

Conversation

davidturissini commented Oct 3, 2018

Details

Does this PR introduce a breaking change?

salesforce-best-lwc-internal bot commented Oct 3, 2018

Benchmark results

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

diervo left a 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

Choose a reason for hiding this comment

salesforce-best-lwc-internal bot commented Oct 9, 2018

Benchmark results

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