proposal-compartments icon indicating copy to clipboard operation
proposal-compartments copied to clipboard

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

Open kriskowal opened this issue 3 years ago • 2 comments

#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 avatar Jun 14 '22 23:06 kriskowal

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)

kriskowal avatar Jun 22 '22 03:06 kriskowal

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.

guybedford avatar Jun 22 '22 04:06 guybedford