Conversation
📚 Pull Request Stack
Managed by gh-stack |
🦋 Changeset detectedLatest commit: 1cd1246 The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| reexports?: string[] | undefined; | ||
| sha512?: string | undefined; | ||
| } | ||
| | { error: string } |
There was a problem hiding this comment.
Error seems to be missing. Is that deliberate?
There was a problem hiding this comment.
that's the unused one. AFAIR, it represents a deferred error and it is not being exposed via the hook (because it's not actually a module but an explicit representation of a lack of one in case someone attempts to require it)
There was a problem hiding this comment.
(yes, it is deliberate)
| exit: moduleSpecifier, | ||
| }, | ||
| canonicalName: compartmentDescriptor.label, | ||
| canonicalName: moduleSpecifier, |
There was a problem hiding this comment.
I'm not sure this is correct.
My understanding of what the facts are (please correct me if I'm wrong)
- this is only reachable for exit modules
- endo has a less specific meaning for exit modules - they're modules that were not provided in compartment map. In lavamoat they should be limited to node builtins and native modules
- the compartmentDescriptor here is the parent compartment in which the exit module was requested
- moduleSpecifier happens to be compatible with canonicalName but formally is not one
With that, I'd rather have canonicalName be undefined for exit modules or remain the parent label, so that values passed to the hook as canonicalName always have a matching compartment.
There was a problem hiding this comment.
I had considered undefined but this breaks the API in a way I didn't want to deal with.
FWIW, if canonicalName is the "parent" canonicalName (i.e. I revert this change), then we're just right back where we started (sans type changes). I am fine with that.
boneskull
force-pushed
the
boneskull/more-module-source-hook-tests
branch
from
abe2c9b to
eba230b
Compare
boneskull
changed the title
…leSource - Removes unused `ModuleSourceHook`'s "error" module source type. - Adds tests for specific behavior around exit modules - Fix a JSDoc import
boneskull
force-pushed
the
boneskull/more-module-source-hook-tests
branch
from
eba230b to
1cd1246
Compare
3 participants
ModuleSourceHook's "error" module source type.