Skip to content

test(wtr): enable custom elements registry tests @W-19097891#5509

Merged
wjhsf merged 75 commits intomasterfrom
wjh/wtr-spam
Sep 24, 2025
Merged

test(wtr): enable custom elements registry tests @W-19097891#5509
wjhsf merged 75 commits intomasterfrom
wjh/wtr-spam

Conversation

@wjhsf
Copy link
Contributor

@wjhsf wjhsf commented Sep 23, 2025

Details

To cleanly test how LWC behaves with the custom elements registry, the tests run in iframes. Getting the right code to run in the iframes had some karma-specific logic that needed to be changed, and I tried to modernize the code (async/await) and make it a bit more clear what is going on.

Does this pull request introduce a breaking change?

  • 😮‍💨 No, it does not introduce a breaking change.
  • 💔 Yes, it does introduce a breaking change.

Does this pull request introduce an observable change?

  • 🤞 No, it does not introduce an observable change.
  • 🔬 Yes, it does include an observable change.

GUS work item

wjhsf and others added 30 commits September 2, 2025 18:01
There's no setup/teardown needed, it's a single test, and WTR provides per-file encapsulation
working toward just importing and executing things, but not quite there yet
two env vars for the same goal is unnecessary
I think the last one was a concurrency related timeout,
which was previously addressed.
helper files shouldn't have side effects; all setup should be in the setup file
all tests are run in isolation
@wjhsf wjhsf requested a review from a team as a code owner September 23, 2025 18:47
Copy link
Member

@jmsjtu jmsjtu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, just a few questions

// of us to use it it directly.
// '../'.repeat(depth) becomes `/__wds-outside-root__/${depth}/`
const depth = outside.length / 3;
return `/__wds-outside-root__/${depth}/${filepath}`;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wjhsf does this mean that @web/dev-server will handle appending the ../ for us?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/__wds-outside-root__/ is a magic path provided by @web/dev-server as a workaround for the fact that you can't ../ out of a web root (in our case, packages@/lwc/integration-not-karma).

When we serve pages with code like import * as LWC from 'lwc', @web/dev-server transforms that import before serving because node-style imports don't work in the browser. First it resolves the path, in this case to ../lwc/index.js. But that import wouldn't work, so it gets transformed again to /__wds-outside-root__/1/lwc/index.js. And then when the browser requests that file, the server knows where to look.

That's fine and dandy for regular imports, but sometimes we do weird hacky nonsense that needs the same transformation, but @web/dev-server doesn't do it for us. So we do it ourselves.


return `(() => {
globalThis.process = { env: { NODE_ENV: "production" } };
globalThis.LWC = globalThis.exports = {};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wjhsf what's this line used for?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LWC is loaded as a CommonJS module, so it must have exports defined in order to work. And then we want to save those exports as a global variable.

}

const LWC = window[globalLWC];
const lwc = window[globalLWC];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: prefer all-caps for globals?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this situation, globalThis.LWC exists, but is not necessarily the same as the local variable. I intentionally used a different name to avoid shadowing.

wjhsf and others added 2 commits September 24, 2025 01:18
@wjhsf wjhsf enabled auto-merge (squash) September 24, 2025 01:57
@wjhsf wjhsf merged commit 8fdac07 into master Sep 24, 2025
7 checks passed
@wjhsf wjhsf deleted the wjh/wtr-spam branch September 24, 2025 02:26
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.

4 participants