-
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
more typedoc #1928
more typedoc #1928
Conversation
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.
I am concerned something marked as "typedoc" is regressing types
@@ -4,16 +4,16 @@ export {}; | |||
/** | |||
* @template Slot | |||
* @callback ConvertValToSlot | |||
* @param {import('@endo/pass-style').PassableCap} val | |||
* @param {any} val a PassableCap |
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 seems like a terrible regression, possibly due to types not being built!
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.
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.
Then the issue is with the PassableCap
definition, not this param definition.
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.
There was indeed an issue with the PassableCap definition. It was any
. The type is still any
so there's no regression here.
Once we fix those types, I expect to return to this.
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.
Fix in #1933
import * as url from 'url'; | ||
import * as process from 'process'; | ||
import * as fs from 'fs'; |
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.
What is the significance of this change? Why didn't it cause anything else to change?
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.
These Node modules have no default exports. Under CommonJS the set of named exports could be treated as an object but under ESM you have to bundle the named exports into an object if you want to access them that way.
@@ -6,6 +6,11 @@ import { makeMarshal } from '../src/marshal.js'; | |||
|
|||
const { freeze, isFrozen, create, prototype: objectPrototype } = Object; | |||
|
|||
const harden = /** @type {import('ses').Harden & {isFake: boolean}} */ ( |
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.
Would the following also work? If so, I think it better documents the runtime reality it describes. Likewise for the other occurrences of course
const harden = /** @type {import('ses').Harden & {isFake: boolean}} */ ( | |
const harden = /** @type {import('ses').Harden & {isFake?: boolean}} */ ( |
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.
Yes it would. I'll see about adding tacking that onto another PR.
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.
Description
Enables typedoc for these packages:
realpathSync
argumentany
andstring
types that it was trying to import frompass-style
but failingSecurity Considerations
Scaling Considerations
Documentation Considerations
Testing Considerations
Upgrade Considerations