Use window object in init function only if necessary for SSR

kant01ne commented 3 years ago

There is no window object when app is rendered server side (SSR) See https://stackoverflow.com/questions/55151041/window-is-not-defined-in-next-js-react-app

Server side rendering using NextJS is impossible for supertokens-auth-react and/or for supertokens-website because of the following line which we could easily optimise https://github.com/supertokens/supertokens-website/blob/3fe92cbede49453c699ab13da219c8904ae745fb/lib/ts/utils.ts#L184

Currently, using SuperTokens with NextJS is suboptimal as we need SuperTokens to be initialised to render logged in or authentication components, but we can only initialise SuperTokens on the client side. This defeats the purpose of using NextJS for SSR.

By making the use of window object optional (if else statement on sessionScope object), we could init SuperTokens completely on the server side.

Note: This is the error that the app encounter during init but there might be others, I haven't checked all use of window object in Session.init() yet

kant01ne commented 3 years ago

Closing as there is actually no point having supertokens-website to be used server side anyway. The fix (the recognised good practice I should say), is to conditionally initialise SuperTokens if window is defined when doing SSR. This will be mentioned in the guide.

rishabhpoddar commented 3 years ago

Is to conditionally initialise SuperTokens if window is defined when doing SSR

This means that it's upto the user to initialise it or not? If yes, then I prefer to know have the user have to do this cause it's yet another thing to communicate (and most likely will be missed)

kant01ne commented 3 years ago

We can add a check for window object at the beginning of the init function.

Pros:

It removes the type check line from the guide.

Cons:

We are adding custom code that's only for SSR support in our client side package. Any side effects we didn't think of?

NextJS is great in theory but 100% of modern client-side packages have not been designed with SSR in mind. It means that someone using SSR is going to have those window typechecks everywhere in his code and it's going to become a pattern for them. Maybe extract all those client side only libs to one file? So I don't think it actually brings any complexity but to be extra safe we can do it.

Wdyt?

Link to the draft of the guide for reference: Readme of https://github.com/NkxxkN/supertokens-nextjs-demo (In progress)

rishabhpoddar commented 3 years ago

So i prefer to achieve two outcomes:

There should not be any complexity (from user's point of view) to implement us for SSR
for future changes to the libs, the complexity to the user should ideally not increase at all.

I leave it up to you to decide what's best. If we are going to specify it in docs, we need to make sure it's easily findable for people who use SSR.

We should also add good error messages in the lib wherever we are using the window object in case it does not exist. The error message should clearly indicate that if you are using SSR, then you need to do ... in order to fix this error.

kant01ne commented 3 years ago

Error messages:

In supertokens-auth-react in `getInstanceOrThrow()`

If window is defined: SuperTokens must be initialized before calling this method.

If window is undefined: SuperTokens must be initialized before calling this method. If you are trying to use this method doing server-side-rendering, please make sure you move this call inside a componentDidMount method or useEffect hook.

Anytime the window object is accessed and not available in both supertokens-auth-react and supertokens-website:

If you are using this package with server-side rendering, please make sure that you are checking if the window object is defined.

kant01ne commented 3 years ago

This was fixed in supertokens-auth-react and in https://github.com/supertokens/supertokens-website/commits/master

supertokens / supertokens-website