feat(mock-doc): add simple MockEvent#composedPath() impl#3204
Merged
rwaskiewicz merged 1 commit intomainfrom Jan 14, 2022
Merged
feat(mock-doc): add simple MockEvent#composedPath() impl#3204rwaskiewicz merged 1 commit intomainfrom
rwaskiewicz merged 1 commit intomainfrom
Conversation
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()
4ca1993 to
c77eae6
Compare
ltm
approved these changes
Jan 14, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull request checklist
Please check if your PR fulfills the following requirements:
npm run build) was run locally and any changes were pushednpm test) were run locally and passednpm run test.karma.prod) were run locally and passednpm run prettier) was run locally and passedPull request type
Please check the type of change your PR introduces:
What is the current behavior?
Users are required to polyfill
composedPathin their tests - e.g. Calcite component testsGitHub 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?
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 rannpm cito 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