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

Refactor Approach to SuperTokens Inputs and Outputs to Be More Flexible #5

Merged

Conversation

ITenthusiasm
Copy link
Owner

@ITenthusiasm ITenthusiasm commented Jan 12, 2023

Refactor Approach to SuperTokens Inputs and Outputs

(This PR has only been created for historical purposes. This branch should be merged locally.)

The SuperTokensInput and SuperTokensOutput utility classes are intended to provide ideas to the SuperTokens team regarding how they can update their functions to be more flexible. This flexibility will enable their functions to be used across various SSR frameworks with ease. These utility classes represent the inputs which these functions should accept (if any are needed) and the outputs which these functions should return (if any are needed), respectively. As potential improvements to what SuperTokens could use for inputs and outputs are discovered, so we ought to update the utility classes that we have here in this repo. Thus, we're making our first refactor here.

Changes

Expose SuperTokens's Response Cookies Explicitly

After chatting with Rich Harris (creator of Svelte) on sveltejs/kit#8409, it seems that it's better to expose any necessary cookies explicitly rather than wrap them in Headers. The short reasoning for this is that it provides more options to developers using SSR frameworks. (The previous implementation locked some developers out of using SuperTokens in the framework of their choice -- for instance, in Svelte Kit.)

We've chosen to expose the output cookies as a Map, where the key is the cookie's name (as a string) and the value is the entire cookie value (as a string). This means that the value returned from Map.get includes name=value and all the cookie's attributes.

Why Expose the Response Cookie value As a string Instead of an object?

Exposing the entire cookie value as a string is arguably the best mix between versatility and ease of use. Most developers will simply be calling something like headers.append("Set-Cookie", cookieValueString) or something similar to meet their needs. This is a nice and simple developer experience. And because the cookie value string is standardized, developers can reliably convert the string into an object if such a conversion is necessary for their SSR framework.

Exposing the cookie value as an object is not as reliable as it seems. First of all, there is no standard object representation of cookies. (Even the popular cookie npm package does not use a consistent object representation between its cookie.serialize and cookie.parse helper functions.) This means that even if we exposed the cookie value as an object, the end-user would still potentially need to convert that object into another object representation. This, of course, is bothersome -- especially if the object key mapping is convoluted.

Second, exposing the cookie value as an object would require many developers to convert the object to a string (for their response headers). And any time SuperTokens changes this object representation (for instance, to match the most popular object representation of cookies in the future), it would result in a breaking change.

Since cookie values will only be interacted with one-at-a-time, it is sufficient for developers to derive each individual cookie's object representation (if one is needed) from the string value based on the location of the ; and = characters.

Why Use Map<string, string> for the Response Cookies Instead of string[]?

Storing the cookies in a Map provides more options to the developer than storing the cookies in an Array. Like an Array, a Map is still Iterable, and useful helpers like forEach can be used reliably with it. Beyond this, the Map can be converted to an Array with Array.from if necessary. But unlike an Array, a Map provides developers (and SuperTokens) with a simple, reliable way to get (or set or reset) a given cookie by name. Attempting to do this with an Array would require much more effort.

A Map was also chosen over a regular object since Maps are inherently iterable.

Expose Reponse Headers As a Map<string, string | string[]>

The responseHeaders are now represented as a Map, where the key is the header's name (as a string in the exact casing of the HTTP header), and the value is either a string for a single value or a string[] for a header with multiple values (i.e., a header that should be returned in the response multiple times).

When leveraging this Map to set the headers of a true Headers object, developers will need to do something like the following:

responseHeaders.forEach((value, name) => {
  if (typeof value === "string") headers.set(name, value);
  else value.forEach((v) => headers.append(name, v));
});

To reduce code duplication, developers can abstract this logic into a helper function.

Why Use Map<string, string | string[]> for the Response Headers Instead of Headers?

Unfortunately, the standard Web Headers API is not reliably supported in Node.js at the moment. Moreover, not every SSR framework is guaranteed to support this representation of headers to begin with. Thus, at the moment, it's simpler and more reliable to expose a common iterable object that all Node.js applications can use with relative ease. And the iterable object that's closest to Headers is arguably the Map.

Moreover, our new representation is compatible with Node.js out of the box. The native response.setHeader method expects a string for headers that should be set only once and an array for headers that should be set multiple times in the response. Thus, developers using the native features of Node.js could simply do the following:

responseHeaders.forEach((value, name) => response.setHeader(name, value));

Short and sweet. 😄

Require the Request Headers As a Map<string, string>, and Expect the Incoming Headers to Include the Cookies

The meaning of this title is fairly straightforward. No explanation is necessary.

Why Use Map<string, string> for the Request Headers?

We've already addressed why a Map is better than Headers for Node.js. So the greater question here is: "Why require a headers input with a different shape than our own headers output?"

The main idea here is ease of use. We want the outputs that we return to be as clear, flexible, and easy to use as possible. Similarly, we want the inputs that we require to minimize the effort on the developer's end as much as possible. And more than likely, the incoming request headers will have the following representation:

  • For a header with a single value, its value will be a string
  • For a header with multiple values, its value will be a string of comma-separated values.

If we run with these assumptions (which are fairly reliable), then SuperTokens can easily derive the header values on its own, and the developer hardly has to do any work.

For developers whose headers are represented by the Headers object, they can simply do something like the following:

// NOTE: If `SuperTokens` updates their functions, then no class will be instantiated.
// The request headers will simply be passed as a `Map` to `SuperTokens`'s functions as needed.
const input = new SuperTokens.Input({ headers: new Map(requestHeaders) });

Developers using the native features of Node.js would need to do something like the following:

const requestHeadersMap = new Map(Object.entries(request.headers));
requestHeadersMap.set("cookie", requestHeadersMap.get("cookie").join(", "));

// NOTE: If `SuperTokens` updates their functions, then no class will be instantiated.
// The request headers will simply be passed as a `Map` to `SuperTokens`'s functions as needed.
const input = new SuperTokens.Input({ headers: requestHeadersMap });

See the Node.js documentation for more information regarding what the IncomingMessage.headers field looks like. Note that the Web Standard Headers.get method is case-insensitive. In fact, new Map(instanceOfHeadersClass) will automatically normalize all header names by making them lowercase. So as long as the SuperTokens implementation assumes that all incoming request header names are all lowercased, everything will work fine without any additional effort from the developer.

If someone can sufficiently argue that consistency between the shape of the input headers and the shape of the output headers is a better developer experience than the ease of use we've just mentioned, then we can revisit this decision. However, for now, this approach seems to make sense.

Why Not Require the Input Data to Include a Separate Cookies Object?

According to Wikipedia:

Browsers do not include cookie attributes in requests to the server—they only send the cookie's name and value.

This means that we can reliably derive all the request cookies from the request headers. There is, therefore, no reason to require the developer to go through the extra effort of providing cookies separately.

If someone can sufficently argue that consistently requiring cookies for BOTH the input AND the output is a better developer experience than the ease of use we've just mentioned, then we can revisit this decision. However, for now, this approach seems to make sense.

…kensOutput`

After chatting with Rich Harris (creator of Svelte) on
sveltejs/kit#8409, it seems that it's better to expose
any necessary auth cookies _explicitly_ rather than wrap
them in `Headers`. The short reasoning for this is that
it provides more options to developers using SSR
frameworks. (The previous implementation locked some
developers out of using `SuperTokens` in the framework
of their choice.)

Note that the `SuperTokensOutput` utility class now
exposes the `responseHeaders` as a `Map` instead of
a `Headers` object in order to support frameworks
(or servers) that do not have a proper `Headers`
class that they can use. (Node.js doesn't quite
support `Headers` out of the box yet. See the
MDN compatibility table.)
This approach is simply more versatile. Not every SSR
framework will have a `Headers` representation, and
Node.js does not yet support `Headers` reliably
locally.

As you can see in the changes, there's very little
additional effort on our end with this refactor (AND
on the developer's end). So really, this is better for
everyone right now.
@ITenthusiasm ITenthusiasm merged commit 5cdc40b into experiment/custom-supertokens Jan 12, 2023
@ITenthusiasm ITenthusiasm deleted the experiment/custom-supertokens-next branch January 12, 2023 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant