Closed NullVoxPopuli closed 9 months ago
Gonna close this PR because the effort is not worth the benefit.
The changes we want are going to be in the next major anyway, so if folks want the upcoming benefits, they can upgrade and to an overrides/resolution to get the benefit.
The main thing here:
Yeah we discussed on Discord –
WAITERS_NAME
set:
unregister
API is unusably broken if you intend to re-use the same name).register
isn't called eagerly in buildWaiter
. It would be slightly sub-optimal and make things slightly more difficult to debug when the problem does happen (the problem = a package consuming ember-test-waiters
is accidentally duplicated). Personally, I think that is probably acceptable.register
call out to buildWaiter
(or in the constructor of the waiter) and see if anything breaks, and solve both problems at once.Map
on the global that warn/throw when on the .set
method when a conflict is detected.
NOTE: this PR will need to target to the
main
branch as well (which I plan on doing after all the test apps are implemented)Context on "The Plan": https://github.com/emberjs/ember-test-waiters/issues/458
historically, we've only been able to allow one copy of test-waiters in a whole projcet. this was forced via build-time code in the v1-addon's index.js. This added maintenance complexity and prevented the conversion to a v2 addon.
With this runtime injection of global state, we can have both the pre-v2 addon and the v2 addon share the same waiter state. Also, in
@ember/test-waiters@v3
, we can force the existing build-time code to take precedecence with the latestVersion of test-waiters v3 (existing behavior), so there is no risk of old copies of test-waiters not using this runtime highlander code, because those older copies would not be present in the final build of an app.So what does this PR actually do?
globalThis
getGlobal
already existed, so much of this work was already doneWAITERS
have already been handledWAITER_NAMES
to also be on the global context, so we don't create a ton of log spew.NOTES