Skip to content

feat(mock-doc): add simple MockEvent#composedPath() impl#3204

Merged
rwaskiewicz merged 1 commit intomainfrom
composed-path-simple
Jan 14, 2022
Merged

feat(mock-doc): add simple MockEvent#composedPath() impl#3204
rwaskiewicz merged 1 commit intomainfrom
composed-path-simple

Conversation

@rwaskiewicz
Copy link
Copy Markdown
Member

@rwaskiewicz rwaskiewicz commented Jan 13, 2022

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Build (npm run build) was run locally and any changes were pushed
  • Unit tests (npm test) were run locally and passed
  • E2E Tests (npm run test.karma.prod) were run locally and passed
  • Prettier (npm run prettier) was run locally and passed

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Users are required to polyfill composedPath in their tests - e.g. Calcite component tests
GitHub Issue Number: N/A

What is the new behavior?

this commit adds a simplistic implementation of composedPath.
specifically, it inspects a MockEvent's target, and attempts to
climb the dom to provide the path the event propagates. this
implementation does not take into account closed shadow roots, nor
slots. the reason for which is that unlike a browser, which has far more
knowledge of the dom and event phases, stencil has none of this at the
mock-doc level, making a deep implementation difficult. a fuller
featured version may be considered for a future version of stencil

Does this introduce a breaking change?

  • Yes
  • No

Testing

Unit tests were added.

I also built this branch (npm run build && npm pack), then installed it in a local clone of calcite-components. Within the calcite-components directory, I ran npm ci to get the dependencies loaded as they are configured today. From there, I navigated to the test suite using polyfills above and removed the polyfill:

    describe("wires up the associated label", () => {
-      beforeEach(() => {
-        // we polyfill composedPath since Stencil's MockEvent does not support it: https://github.com/ionic-team/stencil/blob/main/src/mock-doc/event.ts#L5-L40
-        CustomEvent.prototype.composedPath = function () {
-          // based on https://gist.github.com/rockinghelvetica/00b9f7b5c97a16d3de75ba99192ff05c
-          if (this.path) {
-            return this.path;
-          }
-          let target = this.target;
-
-          this.path = [];
-          while (target.parentNode !== null) {
-            this.path.push(target);
-            target = target.parentNode;
-          }
-          this.path.push(document, window);
-          return this.path;
-        };
-      });

      /**
       * This util helps simulate calcite-label's behavior since we cannot use the component here
       */
      function wireUpFakeLabel(label: HTMLElement): void {

I then ran the tests (npx stencil --spec) to validate they fail without that polyfill. I then installed my local tarball (npm i <PATH_TO_MY_TARBALL>), and reran the tests - they no longer failed.

Other information

I spent quite a bit of time attempting to get a more spec-compliant version of this implemented (spec). As the commit message states, the amount of work/scope began to creep far beyond what we originally intended here. I'm willing to dedicate more time to a spec-compliant version, but I'm also making the tradeoff of shipping this version (team-willing) in Stencil v2 and the more spec-compliant version in a future major version of Stencil

this commit adds a simplistic implementation of composedPath.
specifically, it inspects a MockEvent's target, and attempts to
climb the dom to provide the path the event propagates. this
implementation does not take into account closed shadow roots, nor
slots. the reason for which is that unlike a browser, which has far more
knowledge of the dom and event phases, stencil has none of this at the
mock-doc level, making a deep implementation difficult. a fuller
featured version may be considered for a future version of stencil

STENCIL-240: Enhance MockEvent to support composedPath()
@rwaskiewicz rwaskiewicz marked this pull request as ready for review January 13, 2022 18:49
@rwaskiewicz rwaskiewicz requested a review from a team January 13, 2022 18:49
@rwaskiewicz rwaskiewicz merged commit 7b47d96 into main Jan 14, 2022
@rwaskiewicz rwaskiewicz deleted the composed-path-simple branch January 14, 2022 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants