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

Add some modules/methods on webgme server module of type-defs #1788

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

pmeijer
Copy link
Contributor

@pmeijer pmeijer commented Nov 4, 2023

This is still very much WIP, but might as well merge it in now.

@pmeijer pmeijer added this to the v2.45.1 milestone Nov 4, 2023
@pmeijer pmeijer requested a review from brollb November 4, 2023 20:33
@pmeijer pmeijer self-assigned this Nov 4, 2023
@pmeijer pmeijer changed the title Add some modules/methods on webgme server module Add some modules/methods on webgme server module of type-defs Nov 4, 2023
@pmeijer pmeijer modified the milestones: v2.45.1, v2.46.0 Nov 4, 2023
Also add compile check of types
@pmeijer pmeijer requested a review from kecso March 19, 2024 20:23
Copy link
Contributor

@brollb brollb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It mostly looks good to me. The only major issue I have is with export type Node = object; (see the comment for more info).

@@ -1574,7 +1681,7 @@ declare namespace GmeClasses {
* @return a dictionary where every the key of every entry is an absolute path of a set owner node.
* The value of each entry is an array with the set names in which the node can be found as a member.
*/
isMemberOf(node: Core.Node): Core.DataObject;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was Core.DataObject again?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's used wrongly throughout, but I think the intention was that it should represent the raw data that maps to a node. In other words the json objects stored in the monodb.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*It was

typings/webgme.d.ts Outdated Show resolved Hide resolved
typings/webgme.d.ts Outdated Show resolved Hide resolved
isReadOnly(): boolean;

}
export type Node = object;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... This isn't great since it will result in the following code no longer throwing a type error:

const notANode = {};
const name = core.getAttribute(notANode, 'name')

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good point and I agree. Do you know if there is a way to define a type w/o specifying anything about it, but at the same time constrain anyone from passing anything that isn't of that type.
These Core.Nodes are only being returned from a core instance.

Copy link
Contributor Author

@pmeijer pmeijer Mar 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One way to work around it could be, but feels like there should be a better way..

  export interface Node {
      _internal: string;
  }

@pmeijer pmeijer removed this from the v2.46.0 milestone Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants