Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question for champions: Share or detach host from guest compartment by default? #65

Open
kriskowal opened this issue Jun 14, 2022 · 2 comments

Comments

@kriskowal
Copy link
Member

kriskowal commented Jun 14, 2022

#50 is not ready to land because it fails to adequately account for the case where a guest compartment needs a fresh static record memo (so it can reload) and also reuse the host’s loader’s load hooks. (I’ve made the mistake of conflating the effects of loadHook and memo sharing.) The options need to be orthogonal and the design directions come in two flavors.

(Please weigh in for ❤️ or 🚀 above, or provide feedback in free form.)

❤️: Share by default (which I suspect to be more common)
🚀: Detach by default (which is generally safer)

  • Adopt, override, or omit resolveHook
    • ❤️ (share by default, explicit detach)
      • adopt:
        • new Compartment()
      • override:
        • new Compartment({ resolveHook })
      • omit:
        • new Compartment({ resolveHook: null })
    • 🚀 (detach by default, explicit share)
      • adopt:
        • new Compartment({ shareResolveHook: true })
      • override:
        • new Compartment({ resolveHook })
      • omit:
        • new Compartment()
  • Adopt, override, or omit loadHook
    • ❤️ (share by default, explicit detach)
      • adopt:
        • new Compartment()
      • override:
        • new Compartment({ loadHook })
      • omit:
        • new Compartment({ loadHook: null })
    • 🚀 (detach by default, explicit share)
      • adopt:
        • new Compartment({ shareLoadHook: true })
      • override:
        • new Compartment({ loadHook })
      • omit:
        • new Compartment()
  • Adopt or detach static module record memo
    • ❤️ (share by default, explicit detach)
      • adopt:
        • new Compartment()
      • detach:
        • new Compartment({ detach: true })
    • 🚀 (detach by default, explicit share)
      • adopt:
        • new Compartment({ shareRecords: true })
      • detach:
        • new Compartment()
  • Adopt host globals or create new globals
    • ❤️ (share by default, detach implied by globals option)
      • adopt:
        • new Compartment()
      • detach:
        • new Compartment({ globals: {} })
    • or ❤️ (share by default, explicit detach required)
      • adopt:
        • new Compartment()
      • detach:
        • new Compartment({ detachGlobals: true })
        • new Compartment({ detachGlobals: true, globals: {x, y z} })
    • 🚀 (detach by default, explicit share)
      • adopt:
        • new Compartment({ shareGlobal: true })
      • detach:
        • new Compartment()
        • new Compartment({ globals: {} })
        • new Compartment({ globals: {x, y z} })

I think I would like to convince this group that option B is generally better, even though detached global by default will be surprising the to majority of users. On the other hand, I suspect that attaching the static module record by default will also have surprising negative effects. We could pursue a hybrid, but I know that my mind at least has a hobgoblin.

Please let me know what you think and I’ll update this change to reflect the general favor of the champion group.

@kriskowal kriskowal changed the title Q: Share or detach host from guest compartment by default? Question for champions: Share or detach host from guest compartment by default? Jun 14, 2022
@kriskowal
Copy link
Member Author

kriskowal commented Jun 22, 2022

Today in the first TC39 Module Harmony call, @littledan expressed a preference for guest compartments to generally default to sharing loader behaviors with the host compartment. This interacts with import maps, if the host-defined loader uses one: If a guest compartment falls through to the host’s load hook from the host-defined compartment/loader (the initial realm’s loader), I take that to mean that the guest would benefit from the host’s import map. (cc @syg)

@guybedford
Copy link
Collaborator

To further enumerate some of the scenarios:

  • If setting a resolveHook but not a loadHook, would the resolution default to host module resolution?
  • If setting a loadHook but not a resolveHook, would the host resolution still apply anyway, since these hooks are entirely indepenent?
  • Even if setting a resolveHook, it can still be useful to have access to the host resolver. I previously proposed a second argument to import.meta.resolve to make all import.meta.resolve instances generic resolvers. I think this would be useful to enable calling out to the host loader
  • If we did get the ability to "call out" to the host loader via a generic import.meta.resolve or otherwise, do we even need the host resolver by default, when it might be as simple as resolveHook: hostResolver? When a clean slate compartment should be the use case prioritised.

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

No branches or pull requests

2 participants