-
Notifications
You must be signed in to change notification settings - Fork 72
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
1488 Passable redux #2238
1488 Passable redux #2238
Changes from all commits
2b4ae1b
7ac7d8b
f39b37d
531c2ac
7d43966
fa59e05
9fd3e4d
9d4bb52
e76d85b
a2898b0
e67e1fa
bd5504b
0787913
6206870
c3cdfe3
e2bfb54
905172e
9222cff
dacd908
5fa54f0
1ae099d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,6 @@ | ||
export * from './src/exo-makers.js'; | ||
|
||
// eslint-disable-next-line import/export -- ESLint not aware of type exports in types.d.ts | ||
export * from './src/types.js'; | ||
|
||
export { GET_INTERFACE_GUARD } from './src/get-interface.js'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
/** @file Empty twin for .d.ts */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
export {}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,10 @@ import { q } from '@endo/errors'; | |
|
||
import { Far, passStyleOf, getInterfaceOf } from '../src/index.js'; | ||
|
||
/** | ||
* @import {PassStyled} from '@endo/pass-style'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would I be right to assume that someday There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think btw, the style it enforces is spaces only for object properties and no spaces for the type import wrapper. I support that. |
||
*/ | ||
|
||
const { create, getPrototypeOf } = Object; | ||
|
||
// this only includes the tests that do not use liveSlots | ||
|
@@ -129,6 +133,7 @@ test('passStyleOf validation of remotables', t => { | |
t.throws(() => passStyleOf(badRemotableProto3), NON_METHOD); | ||
t.throws(() => passStyleOf(badRemotableProto4), NON_METHOD); | ||
|
||
// @ts-expect-error UNTIL https://github.com/microsoft/TypeScript/issues/38385 | ||
t.is(passStyleOf(sub(goodRemotableProto)), 'remotable'); | ||
|
||
t.throws(() => passStyleOf(sub(badRemotableProto1)), EXPECTED_PASS_STYLE); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,11 +21,12 @@ import { | |
makeEncodeToSmallcaps, | ||
} from './encodeToSmallcaps.js'; | ||
|
||
/** @import {ConvertSlotToVal, ConvertValToSlot, FromCapData, MakeMarshalOptions, ToCapData} from './types.js' */ | ||
/** @import {Passable} from '@endo/pass-style' */ | ||
/** @import {InterfaceSpec} from '@endo/pass-style' */ | ||
/** @import {Encoding} from './types.js' */ | ||
/** @import {RemotableObject as Remotable} from '@endo/pass-style' */ | ||
/** | ||
* @import {ConvertSlotToVal, ConvertValToSlot, FromCapData, MakeMarshalOptions, ToCapData} from './types.js'; | ||
* @import {Passable, PassableCap, RemotableObject} from '@endo/pass-style'; | ||
* @import {InterfaceSpec} from '@endo/pass-style'; | ||
* @import {Encoding} from './types.js'; | ||
*/ | ||
|
||
const { defineProperties } = Object; | ||
const { isArray } = Array; | ||
|
@@ -78,7 +79,7 @@ export const makeMarshal = ( | |
const slotMap = new Map(); | ||
|
||
/** | ||
* @param {Remotable | Promise} passable | ||
* @param {PassableCap} passable | ||
* @returns {{index: number, repeat: boolean}} | ||
*/ | ||
const encodeSlotCommon = passable => { | ||
|
@@ -134,7 +135,7 @@ export const makeMarshal = ( | |
|
||
if (serializeBodyFormat === 'capdata') { | ||
/** | ||
* @param {Passable} passable | ||
* @param {PassableCap} passable | ||
* @param {InterfaceSpec} [iface] | ||
* @returns {Encoding} | ||
*/ | ||
|
@@ -148,9 +149,11 @@ export const makeMarshal = ( | |
} | ||
}; | ||
|
||
/** @type {(promise: RemotableObject, encodeRecur: (p: Passable) => Encoding) => Encoding} */ | ||
const encodeRemotableToCapData = (val, _encodeRecur) => | ||
encodeSlotToCapData(val, getInterfaceOf(val)); | ||
|
||
/** @type {(promise: Promise, encodeRecur: (p: Passable) => Encoding) => Encoding} */ | ||
const encodePromiseToCapData = (promise, _encodeRecur) => | ||
encodeSlotToCapData(promise); | ||
|
||
|
@@ -184,7 +187,7 @@ export const makeMarshal = ( | |
} else if (serializeBodyFormat === 'smallcaps') { | ||
/** | ||
* @param {string} prefix | ||
* @param {Passable} passable | ||
* @param {PassableCap} passable | ||
* @param {InterfaceSpec} [iface] | ||
* @returns {string} | ||
*/ | ||
|
@@ -231,19 +234,20 @@ export const makeMarshal = ( | |
}; | ||
|
||
const makeFullRevive = slots => { | ||
/** @type {Map<number, Passable>} */ | ||
/** @type {Map<number>} */ | ||
const valMap = new Map(); | ||
|
||
/** | ||
* @param {{iface?: string, index: number}} slotData | ||
* @returns {Remotable | Promise} | ||
* @returns {PassableCap} | ||
*/ | ||
const decodeSlotCommon = slotData => { | ||
const { iface = undefined, index, ...rest } = slotData; | ||
ownKeys(rest).length === 0 || | ||
Fail`unexpected encoded slot properties ${q(ownKeys(rest))}`; | ||
if (valMap.has(index)) { | ||
return valMap.get(index); | ||
const extant = valMap.get(index); | ||
if (extant) { | ||
Comment on lines
+248
to
+249
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, when the value is necessarily truthy, this pattern is better. (Likewise |
||
return extant; | ||
} | ||
// TODO SECURITY HAZARD: must enfoce that remotable vs promise | ||
// is according to the encoded string. | ||
|
@@ -280,11 +284,13 @@ export const makeMarshal = ( | |
const dName = decodeRecur(name); | ||
const dMessage = decodeRecur(message); | ||
// errorId is a late addition so be tolerant of its absence. | ||
const dErrorId = errorId && decodeRecur(errorId); | ||
typeof dName === 'string' || | ||
Fail`invalid error name typeof ${q(typeof dName)}`; | ||
typeof dMessage === 'string' || | ||
Fail`invalid error message typeof ${q(typeof dMessage)}`; | ||
const dErrorId = /** @type {string} */ (errorId && decodeRecur(errorId)); | ||
if (typeof dName !== 'string') { | ||
throw Fail`invalid error name typeof ${q(typeof dName)}`; | ||
} | ||
if (typeof dMessage !== 'string') { | ||
throw Fail`invalid error message typeof ${q(typeof dMessage)}`; | ||
} | ||
const errConstructor = getErrorConstructor(dName) || Error; | ||
const errorName = | ||
dErrorId === undefined | ||
|
@@ -340,8 +346,8 @@ export const makeMarshal = ( | |
const makeDecodeSlotFromSmallcaps = prefix => { | ||
/** | ||
* @param {string} stringEncoding | ||
* @param {(e: unknown) => Passable} _decodeRecur | ||
* @returns {Remotable | Promise} | ||
* @param {(e: unknown) => PassableCap} _decodeRecur | ||
* @returns {RemotableObject | Promise} | ||
*/ | ||
return (stringEncoding, _decodeRecur) => { | ||
assert(stringEncoding.charAt(0) === prefix); | ||
|
@@ -364,7 +370,9 @@ export const makeMarshal = ( | |
}; | ||
|
||
const reviveFromSmallcaps = makeDecodeFromSmallcaps({ | ||
// @ts-ignore XXX SmallCapsEncoding | ||
decodeRemotableFromSmallcaps, | ||
// @ts-ignore XXX SmallCapsEncoding | ||
decodePromiseFromSmallcaps, | ||
decodeErrorFromSmallcaps, | ||
}); | ||
|
@@ -396,7 +404,7 @@ export const makeMarshal = ( | |
// which should be considered fixed once we've completed the switch | ||
// to smallcaps. | ||
assertPassable(result); | ||
return result; | ||
return /** @type {PassableCap} */ (result); | ||
}; | ||
|
||
return harden({ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That’s unfortunate and might explain some of my experience.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah. I think we should consider disabling the rule. I think it has more false positives and fewer true positives than TS