fix(mock-doc): overwrite parentElement in MockHTMLElement to return null #5336
Conversation
|
| Path | Error Count |
|---|---|
| src/dev-server/index.ts | 37 |
| src/mock-doc/serialize-node.ts | 36 |
| src/dev-server/server-process.ts | 32 |
| src/compiler/prerender/prerender-main.ts | 22 |
| src/testing/puppeteer/puppeteer-element.ts | 22 |
| src/runtime/client-hydrate.ts | 20 |
| src/screenshot/connector-base.ts | 19 |
| src/runtime/vdom/vdom-render.ts | 17 |
| src/compiler/config/test/validate-paths.spec.ts | 16 |
| src/dev-server/request-handler.ts | 15 |
| src/compiler/prerender/prerender-optimize.ts | 14 |
| src/compiler/sys/stencil-sys.ts | 14 |
| src/compiler/transpile/transpile-module.ts | 14 |
| src/sys/node/node-sys.ts | 14 |
| src/compiler/prerender/prerender-queue.ts | 13 |
| src/compiler/sys/in-memory-fs.ts | 13 |
| src/runtime/connected-callback.ts | 13 |
| src/runtime/set-value.ts | 13 |
| src/compiler/output-targets/output-www.ts | 12 |
| src/compiler/transformers/test/parse-vdom.spec.ts | 12 |
Our most common errors
| Typescript Error Code | Count |
|---|---|
| TS2345 | 366 |
| TS2322 | 362 |
| TS18048 | 224 |
| TS18047 | 99 |
| TS2722 | 37 |
| TS2532 | 26 |
| TS2531 | 23 |
| TS2454 | 14 |
| TS2790 | 11 |
| TS2352 | 10 |
| TS2769 | 8 |
| TS2538 | 8 |
| TS2344 | 6 |
| TS2416 | 6 |
| TS2493 | 3 |
| TS18046 | 2 |
| TS2684 | 1 |
| TS2430 | 1 |
Unused exports report
There are 14 unused exports on this PR. That's the same number of errors on main, so at least we're not creating new ones!
Unused exports
| File | Line | Identifier |
|---|---|---|
| src/runtime/bootstrap-lazy.ts | 21 | setNonce |
| src/screenshot/screenshot-fs.ts | 18 | readScreenshotData |
| src/testing/testing-utils.ts | 198 | withSilentWarn |
| src/utils/index.ts | 145 | CUSTOM |
| src/utils/index.ts | 269 | normalize |
| src/utils/index.ts | 7 | escapeRegExpSpecialCharacters |
| src/compiler/app-core/app-data.ts | 25 | BUILD |
| src/compiler/app-core/app-data.ts | 115 | Env |
| src/compiler/app-core/app-data.ts | 117 | NAMESPACE |
| src/compiler/fs-watch/fs-watch-rebuild.ts | 123 | updateCacheFromRebuild |
| src/compiler/types/validate-primary-package-output-target.ts | 61 | satisfies |
| src/compiler/types/validate-primary-package-output-target.ts | 61 | Record |
| src/testing/puppeteer/puppeteer-declarations.ts | 485 | WaitForEventOptions |
| src/compiler/sys/fetch/write-fetch-success.ts | 7 | writeFetchSuccessSync |
PR built and packed!Download the tarball here: https://github.com/ionic-team/stencil/actions/runs/7835829975/artifacts/1231522755 If your browser saves files to |
There was a problem hiding this comment.
Are there any instructions for verifying this against the reproduction case?
I installed a locally built tarball in the repro and got:
npm t
> testing-library-test@0.0.1 test
> stencil test --spec
[19:31.2] @stencil/core
[19:31.4] [LOCAL DEV] v4.12.1-dev.1707311711.2f2e3e3 😸
[19:31.4] testing spec files
[19:31.4] jest args: --spec --max-workers=8
FAIL src/components/my-component/my-component.spec.ts
my-component
✕ renders (8 ms)
● my-component › renders
TypeError: Right-hand side of 'instanceof' is not an object
14 | const button = getByRole(root, 'button');
15 |
> 16 | expect(button).toBeInTheDocument();
| ^
17 | });
18 |
19 | });
at checkHtmlElement (node_modules/@testing-library/jest-dom/dist/matchers-57b266e9.js:85:19)
at Object.toBeInTheDocument (node_modules/@testing-library/jest-dom/dist/matchers-57b266e9.js:274:5)
at __EXTERNAL_MATCHER_TRAP__ (node_modules/expect/build/index.js:325:30)
at Object.throwingMatcher [as toBeInTheDocument] (node_modules/expect/build/index.js:326:15)
at Object.<anonymous> (src/components/my-component/my-component.spec.ts:16:20)
Test Suites: 1 failed, 1 total
Tests: 1 failed, 1 total
Snapshots: 0 total
Time: 0.425 s, estimated 1 s
Ran all test suites.
This insinuates there's some issue pulling in that toBeInTheDocument, rather than an issue with the fix here - curious if you're seeing that as well
I haven't. I pushed a reproducible example and added some instructions to run it in the PR description. |
|
That works for me - I modified your repro ever so slightly to align with the original one: diff --git a/src/components/my-component/my-component.spec.ts b/src/components/my-component/my-component.spec.ts
index f846ab7..76dc898 100644
--- a/src/components/my-component/my-component.spec.ts
+++ b/src/components/my-component/my-component.spec.ts
@@ -1,16 +1,16 @@
import { newSpecPage } from '@stencil/core/testing';
-import { screen } from '@testing-library/dom';
+import { getByRole } from '@testing-library/dom';
import '@testing-library/jest-dom';
import { TestComponent } from './my-component';
describe('test component', () => {
it('can find button using getByRole', async () => {
- await newSpecPage({
+ const {root} = await newSpecPage({
components: [TestComponent],
html: `<test-component/>`,
});
- const button = screen.getByRole('button');
+ const button = getByRole(root,'button');
expect(button).toEqualHtml('<button>test</button>');
});
});
and this works as expected. If there's a separate issue with using the aforementioned matcher with Stencil, we should open a new issue. |
|
@rwaskiewicz @tanner-reits thanks for the review, I was able to revert all test changes with some further updates on the event handler. |
What is the current behavior?
MockHTMLElementreturnsMockDocumentwhen accessingparentElement.fixes #5252
STENCIL-1104
What is the new behavior?
Accessing
parentElementin from the<html />node always returnsnullas defined by the spec.According to the spec:
The HTML document is not considered to be an element.
Documentation
Does this introduce a breaking change?
Testing
Added a simple unit test for this. I also created a reproducible example based on the original issue:
git clone git@github.com:christian-bromann/getbyrole-repro.gitcd getbyrole-repronpm installnpm testThe test will pass. The project uses a Stencil dev build
4.12.1-dev.1707312078.2f2e3e3based on this PR.Other information
n/a