fix(mock-doc): improve error message when :scope selector is used

[alicewriteswrongs]

This improves the error message shown when there is an error on the jQuery side when running querySelector and friends by selectively making an addendum to the error message to just explain a bit better what's going on.

STENCIL-1108

What is the current behavior?

If you try to use the :scope pseudo-selector you'll get an error message which is a little less than helpful.

What is the new behavior?

This adds to that error message by adding a try / catch around jQuery.find and in cases where 1) an error was thrown and 2) the selector used has :scope in it we prepend some explanatory text to the error message before re-throwing it.

Documentation

Does this introduce a breaking change?

• Yes
• No

Testing

I added some unit tests which exercise this functionality and check for the right error message.

It's also a good idea to check out how this works in Framework.

You can add the following test to core/src/components/accordion/test/accordion.spec.ts:

it('group should properly set readonly on child accordions', async () => {
  const page = await newSpecPage({
    components: [Item, Accordion, AccordionGroup],
    html: `
      <ion-accordion-group animated="false">
        <ion-accordion>
          <ion-item slot="header">Label</ion-item>
          <div slot="content">Content</div>
        </ion-accordion>
      </ion-accordion-group>
    `,
  });

  const accordionGroup = page.body.querySelector('ion-accordion-group')!;
  // This causes the exception, since it invokes underlying implementation code that uses the :scope selector
  accordionGroup.readonly = true;
});

on @stencil/core@latest this will let you then reproduce the less-than-helpful message. If you then build and install this branch you can check out what the improved version looks like.

[github-actions]

--strictNullChecks error report

Typechecking with --strictNullChecks resulted in 1207 errors on this branch.

That's the same number of errors on main, so at least we're not creating new ones!

reports and statistics

Our most error-prone files

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

[rwaskiewicz]

Looks good! I had 2 non-blocking comments and one larger-picture question. LMK what you think

[rwaskiewicz]

I think there are a couple other L4 selectors that Sizzle doesn't handle on its own right now - :where() and :is(). What do you think about expanding this to query a list of known selectors?

I threw together a rough little patch since it'd take a little change on a couple of different lines/make it hard to look at in GH. TAL and let me know what you think!
multi-l4-selector.patch

[alicewriteswrongs]

makes sense - just pushed that change

[christian-bromann]

Besides what has been already mentioned, LGTM 👍

Thanks for adding additional documentation for selectOne and selectAll 👌

[rwaskiewicz]

LGTM and is working as expected. I had one non-blocking ask for one more assertion in our tests. Otherwise, should be good to go

[rwaskiewicz]

Can we add an assertion for matches as well?

Suggested change

	expect(() => doc.matches(selector)).toThrow(expectedMessage);

[alicewriteswrongs]

oh good idea 👍

[rwaskiewicz]

Ah, we can also probably consider this a 'fix', rather than a chore (and dock -> doc in the PR summary/final commit)

[alicewriteswrongs]

I always write / think mock-dock haha, as if we're building some sort of boat simulator 🚢 ⚓️ 🌊