feat(ses): Compartment options migration #2386
Conversation
packages/ses/src/error/assert.js
Outdated
| // Internal, to obviate polymorphic dispatch, but may become rigorously | ||
| // consistent with @endo/error. | ||
| export { makeError, note as annotateError, redactedDetails as X, quote as q }; | ||
| // consistent with @endo/error: | ||
|
|
||
| /** @type {AssertionFunctions['typeof']} */ | ||
| const assertTypeof = assert.typeof; | ||
|
|
||
| export { | ||
| assertTypeof, | ||
| makeError, | ||
| note as annotateError, | ||
| redactedDetails as X, | ||
| quote as q, | ||
| }; |
There was a problem hiding this comment.
No, this was my preferred solution to avoid a polymorphic dispatch at assertTypeof elsewhere in this change. You had good feedback elsewhere which will change this to assertEqual if not a more systematic change.
| ) { | ||
| const { __options__, ...options } = args[0]; | ||
| assert( | ||
| __options__ === true, |
There was a problem hiding this comment.
since you require it to have value true, you should be sure to mention that where you explain __option__, such as the doc-comment above. From the doc-comment alone, I would guest that you only tested whether the property is present.
packages/ses/src/compartment.js
Outdated
| assertTypeof( | ||
| options.modules, | ||
| 'undefined', |
There was a problem hiding this comment.
Are you sure you want to check that the value is undefined rather than whether the property is present?
Why typeof? The above code should be equivalent to
| assertTypeof( | |
| options.modules, | |
| 'undefined', | |
| assertEqual( | |
| options.modules, | |
| undefined, |
but the later says what it means better.
kriskowal
force-pushed
the
kriskowal-compartment-options
branch
from
a299074
to
59327f0
Compare
Refs: #400
Description
To converge with XS in the patterns accepted by SES for Compartment construction, this change introduces a
__options__property to the first argument of the Compartment constructor that indicates that the constructor accepts a single options bag argument and does not receive separate endowments and module map arguments.Security Considerations
None.
Scaling Considerations
None.
Documentation Considerations
This change includes updates for NEWS (with migration instructions and a notice of intent to break deprecated usage patterns) and README (to encourage recommended usage). We intend to follow up with an update to hardenedjs.org upon release.
Testing Considerations
All tests have been updated to use the new constructor pattern except those already marked as legacy, which remain to exercise support for the deprecated usage patterns.
Compatibility Considerations
There is a remote possibility that existing code uses
__options__as the name of an endowment and will now be misinterpreted. We do not imagine any guest programs are in a position to influence their host compartment’s construction.Upgrade Considerations
None.