This isn't relevant to the import precedence that this describe() is testing. These tests verify what happens when there are multiple possible interpretations of the same load. Because the package importer is always absolute (since it has the pkg: scheme), it's never ambiguous with relative, load path, or working-directory loads.

That makes sense. I've removed this test here. I added this as the spec specifies that Package Importers are resolved first for the legacy API. Do we need to test that, or perhaps, is there really a way to test that, as there can't be conflicts?

@nex3 I needed to make this change after merging Shared Resources into node-embedded-host, as all async tests were failing. I think this is correct now (the callback can be async, so we need to await it, so that the finally doesn't happen before the callback resolves).

However, it seemed worth flagging with you because the async dart-sass tests did not fail, and the node-embedded-host tests did not fail until after merging in Shared Resources, and we weren't able to track down any cause for this difference.

It could be that functions with FutureOr<T> return type in dart is not really an async function in JS, but a sync function that may return a Promise or T. This allows the some synchronous code path to be dispatched synchronously, and only code inside the returned Promise is dispatched asynchronously, therefore may lead to different execution order than how a true asynchronous function is dispatched.

This should just test new NodePackageImporter() without passing it to compileString() now.

Addressed in oddbird@b18ff2c