feat: GitHubProject.getInstance(), project.getProperties()

[tmelliottjr]

Adds databaseId and other project level properties to the GitHubProject object.

Closes #66

[gr2m]

We need a separate method to get project data, using getters that return undefined until a certain internal state is not a good API. The new API should always behave the same to the consumer, no matter the internal state.

[tmelliottjr]

We need a separate method to get project data, using getters that return undefined until a certain internal state is not a good API. The new API should always behave the same to the consumer, no matter the internal state.

You're right! I'll rework this. I had something more akin to that originally, but ultimately went with the 'simpler' option here.

[gr2m]

Start with the the README update to show the new API, once we agree on that, the rest will be easy.

[tmelliottjr]

Start with the the README update to show the new API, once we agree on that, the rest will be easy.

Will do, my plan is to start this tomorrow, but I may not get to it until Monday.

[tmelliottjr]

@gr2m Here is the updated readme adding references for the potential new API.

Another path we can consider is making this available during initialization. This might also lend itself to some issues we're trying to solve internally. For example, instantiating with an non-existent project number provides no immediate feedback:

const project = new GitHubProject({
  token: "<token>",
  org: "<org>",
  number: 9999,
});

It's not until we attempt to list items that we realize an issue exists:

await project.items.list()

file:///github-project/api/lib/get-state-with-project-items.js:33
    projectNext.fields.nodes
                ^

TypeError: Cannot read properties of null (reading 'fields')
    at getStateWithProjectItems (file:///github-project/api/lib/get-state-with-project-items.js:33:17)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async listItems (file:///github-project/api/items.list.js:13:26)
    at async file:///github-project/.test/scratchpad.js:12:13

Node.js v18.2.0

[gr2m]

I'd suggest to merge #72 first, then create a 2.x branch, and update this PR head from main to 2.x. I'd add the change to #72.

If we merge it first then I fear it'd add a lot merge conflicts

Here is the updated readme adding references for the potential new API.

looks good 👍🏼

Another path we can consider is making this available during initialization

We could offer a factory method (e.g. a static class method such as Project.get(options)) as an alternative which would internally instantiate the project instance and run project.get().

We should definitely improve error messages either way. For a better debugging experience, we could call project.get() before using any other methods.

Removing the synchronous instantiation altogether will make it harder to test & mock.

[gr2m]

Should we go with Project.get() and project.get()? I fear that might be confusing, as Project.get() would resolve with a project instance, and project.get() would resolve with with an object of project properties.

Maybe Project.getInstance() and project.getProperties()?

[tmelliottjr]

Should we go with Project.get() and project.get()? I fear that might be confusing, as Project.get() would resolve with a project instance, and project.get() would resolve with with an object of project properties.

Maybe Project.getInstance() and project.getProperties()?

Definitely agree, that Project.get() and project.get() would be too confusing. Project.getInstance() and project.getProperties are perfect! I have been referring to the latter as data, but properties seems clearer and wouldn't imply that it also included items.

[gr2m]

Let me know if you plan on working on it. If not I can try to work on it myself, but can't make promises when I'll find the time right now

[tmelliottjr]

Let me know if you plan on working on it. If not I can try to work on it myself, but can't make promises when I'll find the time right now

Yes! I plan to, but I'm working through some higher priority items atm.