Isomorphic SDK support

[pi0]

Hey! Congratulations on public notion API and awesome work on minimal and typed js SDK 💯

As of the current version, this package seems to only work with node.js, which makes missing lots of opportunities for supporting non-nodejs environments such as Deno, Cloudflare workers, universal frameworks like Nuxt.js , and frontend libraries like vue-notion this can open LOTS of new opportunities for using sdk.

For this change, we need to avoid depending on got and using a universal alternative (fetch) API. Since request is already refactored, it would be easy to allow providing custom client implementation or creating multi-target bundles (see ohmyfetch as an example) that importing @notionhq/client be as is now but having @notionhq/client/isomorphic import (or /node import) that is preconfigured with either node-fetch or got or faster node clients like nodejs/undici / undici-fetch

I can propose a PR if above seems reasonable for purpose of notion-sdk.

[taylorjdawson]

@pi0 I agree that is an important feature, however, I am also a huge fan of Sindre Sorhus's work so I would advocate for either:

• a) Just switching from got to ky
• b) Allowing the user to pass in their preferred npm module for requests, for instance:

import ky from 'ky'
const notion = new Client({
  requestClient: ky,
  auth: process.env.NOTION_TOKEN,
});

[b-zurg]

It looks like ky allows providing one's own fetch function compatible with the browser API.

I don't feel like ky provides a huge benefit over fetch - it's not so hard to write one's own wrappers around the API if need be and less abstractions usually means easier debugging in my experience.

I think passing in a fetch function is a good pattern though.

[aoberoi]

thanks for the kind words and for kicking off the discussion. it sounds like there's a few ways we could go about this.

@pi0 i'm not too familiar with all the packaging concerns across Deno, Cloudlfare workers, and the other JS runtimes available. But just from an ES Modules perspective, I'm not sure the multi-target bundle (where we use import specifiers like @notionhq/client/isomorphic or @notionhq/client/node) will limit this package's compatibility or not. Base specifiers are already a non-standard, bad thing, but everyone uses them because of Node. Can you help me understand a little more how many various formats and packages this might involve?

the idea @taylorjdawson put forward seems a bit simpler to me. are there any disadvatanges to that approach? for example, if i'd like to use @notion/client in the browser, am i paying the install cost of got and then need to use some sophisticated tree-shaking build pipeline to get rid of all that code for runtime?

i still see usage on the backend with Node as the primary use case for this package. the Notion API wasn't really designed with client side application uses in mind. but i'm open to some of these ideas because it sounds like there's some exciting possibilities here.

[pi0]

Thanks for inputs @aoberoi @b-zurg @taylorjdawson 🙏

So basically the proposed change is to make library depending on native Fetch API. This works out-of-the-box and without polyfills for almost every modern javascript runtime that already supports whatwg/fetch.

// Works for browser, web-worker, CloudFlare workers and Deno
import { Client } from '@notionhq/client'

const notion = new Client({ })

Well, with exception of NodeJS! [^1] [^2] We can add a new option as of @taylorjdawson suggestion to provide HTTP client (a fetch-compatible library):

// Alternatives: node-fetch, cross-fetch, isomorphic-fetch, undici-fetch
import fetch from 'node-fetch'
import { Client } from '@notionhq/client'

const notion = new Client({ fetch })

// or more advanced with options
// const notion = new Client({ fetch, fetchOptions: { agent } })

But since probably majority of sdk users are NodeJS users, we can also provide a shortcut with preconfigured instance of fetch polyfill:

// Works with NodeJS (and bundler support for browsers)
import { Client } from '@notionhq/client/node'
// or CommonJS
const { Client } = require('@notionhq/client/node')

const notion = new Client({})

---

[^1] Unless it is already global polyfilled. Frameworks like Nuxt.js, do this by default so fetch is available in universal code. [^2] NodeJS is actively working to bring whatwg components into the core. So there would be no surprise eventually we have native fetch in NodeJS too (ref: https://github.com/nodejs/node/issues/19393)

just from an ES Modules perspective will limit this package's compatibility or not

Actually, it should extended package compatibility from NodeJS/CommonJS to universal ESM + NodeJS/ESM+CommonJS + Deno. exports is becoming standard for esm supporting packages. I think what you are referring to is legacy isomorphic approaches (conditional module exports) that is used by libraries like isomorphic-fetch that i fully agree is becoming non-standard.

i still see usage on the backend with Node as the primary use case for this package. the Notion API wasn't really designed with client side application uses in mind

In many ways it makes sense. Isomorphic support allows supporting more backend runtimes like Deno that are probably closer to notion-api usage intention while browser usage might be tricky especially when it comes to passing a token. But if we support a consistent SDK client, we can use methods like using SDK in-browser via a backend proxy to solve security concerns and opening new possibilities.

Just switching from got to ky

I would say It depends on how much of it's features we need. As @aoberoi and @b-zurg mentioned, it probably costs more tree-shaking (and potentially Node runtime dependency). KY-universal is also discouraged by the maintainer.

[aoberoi]

Before we build out support for more JS runtimes, I'd like to understand some of the use cases a bit more. One question I have is, do we really have use cases for Cloudflare workers and/or Deno right now?

I've created #60 to organize some conversation around browser support specifically. I think we might need to set some expectations around that topic because sharing tokens with a browser application is probably something we should not advise until the Notion API's token model is designed for that (no promises).

I don't mind keeping this issue open because it's already captured some great input. @pi0 @taylorjdawson @b-zurg is there a specific target you're most interested in? or are we more interested in increasing the possible adoption of the Notion API proactively?

[taylorjdawson]

@pi0 @b-zurg Thanks for your valuable input! I agree that ky isn't necessary if you don't care about the extra features, although if I was the person maintaining the package I would opt for ky just because I find it speeds up my dev time. But since this is used internally it won't make much difference to me.

@aoberoi I like the idea of creating separate issues for separate runtime support and that browser support should (probably?) be prioritized

If you wanted to go the route of not using a library that is both node & browser compatible then I like @pi0's original idea of having @notionhq/client for node users and @notionhq/client/isomorphic

I would say since this was designed to be used more on the backend that node.js user would have the least amount of friction and let the browser users import something like @notionhq/client/isomorphic or just pass in a fetch function that is browser compatible and overrides the libraries' built in fetch function.

[xaviripo]

@aoberoi I for one look forward to using this SDK with Deno. The use cases for Deno are mostly the same as Node, given that it's supposed to be a replacement (kind of), so anything for server-side JS/TS.

It's just a shame that the incompatibility with browser/Deno/etc. is "just" due to a technical dependency like got. Especially when fetch is the standard.

[justjake]

I took a look at ky, but I didn't see a straightforward way to use it as the basis of isomorphic support. @taylorjdawson suggests providing a HTTP client implementation as an option to Client class, but ky has a large API surface area that makes it unsuitable for this purpose. Furthermore, ky-universal adds global polyfills, which is an unacceptable choice to force onto our library users.

I decided to go with cross-fetch for my PR. This won't address the Deno concerns directly, but does bring us much closer in line with those needs. Thanks to cross-fetch, there's no need for different imports for browser vs node.

[xg-wang]

Hi, thanks for the discussion folks, isomorphic support would be a great addition to the SDK.

I built the node & browser bundles locally, but it looks like notion api does not allow CORS. Here's a 400 response from notion api on my preflight request:

❯ curl 'https://api.notion.com/v1/databases' \
  -X 'OPTIONS' \
  -H 'authority: api.notion.com' \
  -H 'pragma: no-cache' \
  -H 'cache-control: no-cache' \
  -H 'accept: */*' \
  -H 'access-control-request-method: GET' \
  -H 'access-control-request-headers: authorization,notion-version' \
  -H 'origin: http://127.0.0.1:8080' \
  -H 'user-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/93.0.4567.0 Safari/537.36' \
  -H 'sec-fetch-mode: cors' \
  -H 'sec-fetch-site: cross-site' \
  -H 'sec-fetch-dest: empty' \
  -H 'referer: http://127.0.0.1:8080/' \
  -H 'accept-language: en-US,en;q=0.9' \
  --compressed
{"object":"error","status":400,"code":"invalid_request_url","message":"Invalid request URL."}% 

Would it make sense for Notion API to allow CORS so it can be used in the browser directly?

[alfonsodlopez]

@xg-wang - It would not as it poses a security risk by when credentials are exposed in client-side requests.

[gunta]

+1 Came here surprised to see that it doesn't support Cloudflare Workers yet, since Notion API makes the perfect case for doing automation with 0-ms cold boot start server-less platforms.

I vote for having a requestClient parameter for flexibility, but at the same time I think also having support by default (ie. being tested and having official support) for most popular platforms outside Node would be a very important thing for the future of this project (and the growing community).

• Deno
• Cloudflare workers
• Vercel functions
• Universal frameworks

[alvarosabu]

I echo what @pi0 commented, the benefits of enabling an isomorphic version for ESM are highly valuable.

I was trying to fetch a database from my Notion to a Vite + Vue app, and have the same issue as here https://github.com/makenotion/notion-sdk-js/issues/209, that means that I will need to do a handmade version of the client with Axios. Although now getting problems with CORS.

I hope this solutions go forward

[al6x]

+1 for Deno

[huw]

This issue is mostly fixed by the fetch option on the Client constructor—I’ve gotten this working absolutely fine in Cloudflare Workers.

Running into one more issue, though, which I’ll leave here for posterity in case someone else runs into what I had.

The fetch-types.d.ts declaration includes the line <reference lib="dom" />, which imports the dom type definitions for TypeScript. This is convenient in 99% of use-cases, but unfortunately Cloudflare Workers uses cloudflare/workers-types to override the basic webworker types with Cloudflare’s own. In particular, I ran into incompatibilities with Crypto.randomUUID() and Cloudflare’s non-standard additions to the WebSocket protocol (socket.accept() and Response.webSocket).

The best workaround is to patch your node_modules to remove that line, and then everything works fine. This should apply to anyone using TypeScript in a non-standard environment (e.g. Deno) with slightly different typings to the default DOM.

[rajeshg]

This issue is mostly fixed by the fetch option on the Client constructor—I’ve gotten this working absolutely fine in Cloudflare Workers.

Running into one more issue, though, which I’ll leave here for posterity in case someone else runs into what I had.

The fetch-types.d.ts declaration includes the line <reference lib="dom" />, which imports the dom type definitions for TypeScript. This is convenient in 99% of use-cases, but unfortunately Cloudflare Workers uses cloudflare/workers-types to override the basic webworker types with Cloudflare’s own. In particular, I ran into incompatibilities with Crypto.randomUUID() and Cloudflare’s non-standard additions to the WebSocket protocol (socket.accept() and Response.webSocket).

The best workaround is to patch your node_modules to remove that line, and then everything works fine. This should apply to anyone using TypeScript in a non-standard environment (e.g. Deno) with slightly different typings to the default DOM.

Can you provide a small code sample on how you got this working? I am trying to fetch some data from notion

[huw]

@rajeshg Um, sure.

const notion = new Client({ auth: process.env.NOTION_TOKEN, fetch: fetch });

That’s assuming you’re in an environment that has a fetch-API-compatible fetch client. If you’re in Cloudflare Workers, the supplied fetch client is API-compatible, except that the Response object it returns is immutable. So I wrote:

const fetchWithMutableResponse = async (
  url: Parameters<typeof fetch>[0],
  init: Parameters<typeof fetch>[1]
): ReturnType<typeof fetch> => {
  const responseOriginal = await fetch(input, init);
  return responseOriginal.clone();
};

const notion = new Client({ auth: process.env.NOTION_TOKEN, fetch: fetchWithMutableResponse });

(Although in Workers you don’t have process.env.NOTION_TOKEN, so in that case I just supplied NOTION_TOKEN. But you get the idea)

Does that make sense? I was not totally sure what you were asking. If you meant to ask about patching NPM packages, just run yarn patch @notionhq/client and yarn patch-commit --save $PATCH_PATH with your patch path.