fix: undefined check for guestSpaceId of rest-api-client

[the-red]

Why

In our team, we often write code that looks like this

const Apps = {
  isGuestSpace: false,
  spaceId: 32,

  user: 126,
  order: 127,
  billing: 128,
};
const client = new KintoneRestAPIClient({ guestSpaceId: Apps.isGuestSpace ? Apps.spaceId : undefined })

We want you to treat not only undefined, but also null and '' in the same way.

const client = new KintoneRestAPIClient({ guestSpaceId: Apps.isGuestSpace ? Apps.spaceId : null })
const client = new KintoneRestAPIClient({ guestSpaceId: Apps.isGuestSpace ? Apps.spaceId : '' })

What

-  guestSpaceId !== undefined ? xxx : yyy
+  guestSpaceId ? xxx : yyy

How to test

expect(new KintoneRestAPIClient().record.client.guestSpaceId).toBe(undefined)
expect(new KintoneRestAPIClient({ guestSpaceId: undefined }).record.client.guestSpaceId).toBe(undefined)
expect(new KintoneRestAPIClient({ guestSpaceId: null }).record.client.guestSpaceId).toBe(undefined)
expect(new KintoneRestAPIClient({ guestSpaceId: '' }).record.client.guestSpaceId).toBe(undefined)

Checklist

• Read CONTRIBUTING.md
• Updated documentation if it is required.
• Added tests if it is required.
• Passed yarn lint and yarn test on the root directory.

[koba04]

@the-red
Thank you for sending a PR!!! 👏
Could you describe more details about why this change makes sense?

I didn't catch why this change is required. Why do you want to write as { guestSpaceId: Apps.isGuestSpace ? Apps.spaceId : null } rather than { guestSpaceId: Apps.isGuestSpace ? Apps.spaceId : undefined }?

The current implementation is intended to be able to omit guestSpaceId when it is unnecessary, so we expect developers omit guestSpaceId value rather than setting falsy values like null or ''.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[the-red]

I apologize for the late reply 🙏

The current implementation is intended to be able to omit guestSpaceId when it is unnecessary, so we expect developers omit guestSpaceId value rather than setting falsy values like null or ''.

If that is the case, I would like to see an error when calling the constructor.

In the case of null, I still get a TypeScript type error, which is fine.

const client = new KintoneRestAPIClient({ guestSpaceId: null })

But in the case of '', I get neither a type error nor a constructor error.

const client = new KintoneRestAPIClient({ guestSpaceId: ''})
await client.record.getAllRecords({app: kintone.app.getId()})

And when I call the API, I get an error and get confused.

I think one of the following is the natural behavior.

• The same behavior for null, '', and undefined
• In the case of null and '', an error occurs when the constructor is called.

[tasshi-me]

@the-red
Thank you for the response!

I understand the behavior what you are reporting; the rest-api-client generates broken request url and causes a runtime error when invalid guestSpaceId is given.
I have registered this behavior as an issue #917.

We plan to make a fix for this behavior with adding a validation. However, we want to consider the appropriate place for validation. (I guess it will be above section you changed.)
We will continue the discussion in #917.

[zaki-yama]

@the-red
We fixed #917 by adding a validation in constructor.
So if you pass null or '' to guestSpaceId, an error occurs when the constructor is callled.

This fix is already released in v2.0.15.

We'll close this PR, but we really appreciate your feedback!

[the-red]

@zaki-yama Thank you!!!! 🚀