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

Server-side only method to ensure server-only code is never sent to the browser #5354

Closed
Jauny opened this issue Oct 1, 2018 · 25 comments
Closed

Comments

@Jauny
Copy link

Jauny commented Oct 1, 2018

Feature request

Is your feature request related to a problem? Please describe.

Let's say I have a hypothetical Analytics.js module which works differently depending on whether we are on the server or in the browser. It dynamically loads a library-node or library-browser package with an eval for example.

Since it's an analytics module we need it inside most of our components to send events.

I tried to initialise it in getInitialProps like this:

static async getInitialProps({req}) => {
  this.analytics = new Analytics(!!req)
}

This allows to correctly load Analytics with server or client side setup, but when loaded on the server side, the code will then be sent to the browser, which will error on unknown code (such as fs etc).

Describe the solution you'd like

It would be nice to have a way to say "here is some code I want ran only on the server because it will break in the browser".
We can already do this with browser code, because we have componentDidMount which is only called in the browser, so we know it will never get evaluated on the server. Something similar for server-side would be helpful.

Describe alternatives you've considered

Right now I'm just loader my module inside component in componentDidMount (only in the browser) and abstracted the analytics logic in other non-components modules that are ran in the server, but it's not as flexible and mixes concerns.

Additional context

n/a

Thanks!

@HaNdTriX
Copy link
Contributor

HaNdTriX commented Oct 1, 2018

Did you try?

if (!process.browser) {
  console.log('this code will only run on the server')
}

or

let myLib
if (process.browser) {
  myLib = require('library-browser')
} else {
  myLib = require('library-node')
}

@sergiodxa
Copy link
Contributor

process.browser as far as I remember is something from webpack and not safe to use. Using typeof window === 'undefined' will give you the same result.

@Jauny
Copy link
Author

Jauny commented Oct 2, 2018

@HaNdTriX being able to check if I'm in the server or the browser is not really a problem. As you mention we can use process.browser or as sergiodxa mentions check for window.

My issue is that if I do something like this.analytics = new mylib(), when the component is rendered server side, the server version of the lib is correctly loaded, and sent to the browser with it, so then the browser has issues with it.

What I'm offering here is a way to mark code to be loaded/ran during server side rendering, but then maybe purged before it is sent to the browser.

The more I try to explain this is less it seems plausible tho 🤔

@hugotox
Copy link

hugotox commented Oct 6, 2018

can't you move your server logic to the server.js file instead of your component?

@HaNdTriX
Copy link
Contributor

HaNdTriX commented Oct 6, 2018

@sergiodxa As far as I know the behaviour of process.browser was changed in next@7.

So browser.browser will now be picked up by the minifier and remove the code.
Please note that this only works at the root scope and with simple if statements.
In addition please note that you`ll need to use the commonjs syntax (see my example above).

Here is the code thats related to my theory:
https://github.com/zeit/next.js/blob/canary/packages/next/build/webpack.js#L285

Another approach could be to use of dynamic imports (see ssr option).

@pengshaosu
Copy link

pengshaosu commented Nov 22, 2018

process.browser as far as I remember is something from webpack and not safe to use. Using typeof window === 'undefined' will give you the same result.

when window === 'undefined' casuing ReferenceError: window is not defined
window can't be resolved on server side.

@hugotox
Copy link

hugotox commented Nov 22, 2018

@pengshaosu :

typeof window === 'undefined'

@pengshaosu
Copy link

pengshaosu commented Nov 22, 2018

@hugotox

thx.
how can disable getInitialProps call only when next export, still working when next build(both csr and ssr). maybe i can by process.env. any other apporach?

@constb
Copy link

constb commented Nov 25, 2018

@HaNdTriX really knows what he's talking about. #5159 added code elimination on webpack-level when process.browser is referenced. Webpack replaces it with true in client build stage, and with false when compiling server. Then it's just standard dead code elimination routine… This should probably be documented somewhere, but either way maybe we should close this issue?

@codyzu
Copy link

codyzu commented Jan 20, 2019

I found an interesting solution:

  1. serve next using an express server as shown here
  2. require whatever server side modules you need inside your server.js and add them to the req via an express middleware
const myModule = require('this-only-works-on-the-server');

// ... initialize express + next

server.use((req, res, next) => {
  // now I can access this module inside the getInitialProps function
  req.myModule = myModule;
  return next();
})

server.get('*', (req, res) => {
  return handle(req, res);
})

☝️ this avoids webpack ever trying to include the server side module since the server.js file is never imported by webpack.

@musemind
Copy link

Is there a way to use a folder (for example /server/*) for server-only code. My server.js grows fast.

@janoist1
Copy link

janoist1 commented Feb 4, 2019

I found an interesting solution:

  1. serve next using an express server as shown here
  2. require whatever server side modules you need inside your server.js and add them to the req via an express middleware
const myModule = require('this-only-works-on-the-server');

// ... initialize express + next

server.use((req, res, next) => {
  // now I can access this module inside the getInitialProps function
  req.myModule = myModule;
  return next();
})

server.get('*', (req, res) => {
  return handle(req, res);
})

☝️ this avoids webpack ever trying to include the server side module since the server.js file is never imported by webpack.

That's a great idea!!

@jaydenseric
Copy link
Contributor

The idea in #5354 (comment) might be ok within a project, but is not particularly viable for published packages as consumers might not have a custom server setup.

For next-graphql-react I ended up exporting a Next.js plugin that for the client bundle aliases all the server only exports to empty named exports; see withGraphQLConfig.

I also use the undocumented, but handy, process.browser to eliminate client/server code in the relevant bundles: https://github.com/jaydenseric/next-graphql-react/blob/v2.0.0/src/universal/withGraphQLApp.mjs#L85

This used to be inferior to typeof window !== 'undefined' checks as Webpack would include a massive process polyfill, but I'm pretty sure that no longer happens in recent Next.js releases.

@janoist1
Copy link

janoist1 commented Feb 4, 2019

Is there a way to use a folder (for example /server/*) for server-only code. My server.js grows fast.

Yes. Create a folder server move server.js to server/main.js and update your package.json scripts accordingly. For example:

    "dev": "node server/main.js",
    "inspect": "node --inspect server/main.js",
    "start": "NODE_ENV=production node server/main.js",

@codybrouwers
Copy link
Member

For anyone else like me that comes across this thread wondering what they should use, according to this PR #7651 process.browser is now deprecated and the recommended approach is

typeof window === "undefined"

@Timer
Copy link
Member

Timer commented Aug 15, 2019

👆 the above actually results in the code being removed from your client-side bundles.

We have a test to ensure server-only code does not appear in the client bundle: #8361!

@Timer Timer closed this as completed Aug 15, 2019
erbridge added a commit to LBHackney-IT/mat-process-thc that referenced this issue Feb 25, 2020
Next.js has changed the recommended way to detect whether we're on the
client or not to check for the presence of `window`.

See vercel/next.js#5354 (comment).
erbridge added a commit to LBHackney-IT/mat-process-thc that referenced this issue Feb 25, 2020
Next.js has changed the recommended way to detect whether we're on the
client or not to check for the presence of `window`.

See vercel/next.js#5354 (comment).
erbridge added a commit to LBHackney-IT/mat-process-thc that referenced this issue Feb 26, 2020
Next.js has changed the recommended way to detect whether we're on the
client or not to check for the presence of `window`.

See vercel/next.js#5354 (comment).
erbridge added a commit to LBHackney-IT/mat-process-thc that referenced this issue Feb 26, 2020
Next.js has changed the recommended way to detect whether we're on the
client or not to check for the presence of `window`.

See vercel/next.js#5354 (comment).
erbridge added a commit to LBHackney-IT/mat-process-thc that referenced this issue Feb 27, 2020
Next.js has changed the recommended way to detect whether we're on the
client or not to check for the presence of `window`.

See vercel/next.js#5354 (comment).
erbridge added a commit to LBHackney-IT/mat-process-thc that referenced this issue Feb 27, 2020
Next.js has changed the recommended way to detect whether we're on the
client or not to check for the presence of `window`.

See vercel/next.js#5354 (comment).
erbridge added a commit to LBHackney-IT/mat-process-thc that referenced this issue Mar 2, 2020
Next.js has changed the recommended way to detect whether we're on the
client or not to check for the presence of `window`.

See vercel/next.js#5354 (comment).
erbridge added a commit to LBHackney-IT/mat-process-thc that referenced this issue Mar 2, 2020
Next.js has changed the recommended way to detect whether we're on the
client or not to check for the presence of `window`.

See vercel/next.js#5354 (comment).
erbridge added a commit to LBHackney-IT/mat-process-thc that referenced this issue Mar 2, 2020
Next.js has changed the recommended way to detect whether we're on the
client or not to check for the presence of `window`.

See vercel/next.js#5354 (comment).
erbridge added a commit to LBHackney-IT/mat-process-thc that referenced this issue Mar 4, 2020
Next.js has changed the recommended way to detect whether we're on the
client or not to check for the presence of `window`.

See vercel/next.js#5354 (comment).
@y0n3r
Copy link

y0n3r commented Jun 25, 2020

@Timer Will the following check work as well?

const hasWindow = () => {
  return typeof window === 'object'
}

if (!hasWindow()) {
  // server-side code
}

Or does the built-in Next optimization search specifically for when window is undefined?

@timneutkens
Copy link
Member

It specifically replaces typeof window with 'object' or 'undefined' based on the environment. This is then shaken away by Terser because it becomes if(true) or if(false). So yes you have to use the explicit typeof check instead of creating a function for it.

@dan-silk-discovery
Copy link

dan-silk-discovery commented Jun 26, 2020

It specifically replaces typeof window with 'object' or 'undefined' based on the environment. This is then shaken away by Terser because it becomes if(true) or if(false). So yes you have to use the explicit typeof check instead of creating a function for it.

I really hate this answer... no offense! It shouldn't evaluate a function at build time like that without an explicit plugin. If this is in fact what's happening, then we should look into removing webpack all together in favor of pure es6 modules.

Is it possible to prevent this or do somewhat of the reverse: use a plugin to swap a placeholder with hasWindow() to be evaluated at runtime?

@timneutkens
Copy link
Member

timneutkens commented Jun 27, 2020

I really hate this answer... no offense!

Not sure what your problem is with the answer... I've explained the behavior that has been there since Next.js 9.1 or so. Hate is a pretty strong word 😅

It shouldn't evaluate a function at build time like that without an explicit plugin.

It's not evaluating a function, it's converting a statement that is known to always be a certain value because we take care of the full environment.

If this is in fact what's happening, then we should look into removing webpack all together in favor of pure es6 modules.

This is a bundle size / performance optimizations that so far has had no issues reported by issuers. It only runs against first-party code so not against node_modules (would have even bigger size wins if we did run it against node_modules however that gave issues with some packages in the ecosystem).

Is it possible to prevent this or do somewhat of the reverse: use a plugin to swap a placeholder with hasWindow() to be evaluated at runtime?

Why do you want to do that? There is no benefit to it as we know it's always going to be defined client-side.

This is no different than process.env.NODE_ENV being replaced with production or development based on the environment.

We do tons of optimizations against first-party code that make sure the bundle output is as optimized as possible. E.g. getStaticProps and getServerSideProps are dropped from client-side bundles as well: https://next-code-elimination.now.sh/

@dan-silk-discovery
Copy link

dan-silk-discovery commented Jul 9, 2020

Sorry, the "hate" was due to a looming deadline. 😉

We do tons of optimizations against first-party code that make sure the bundle output is as optimized as possible. E.g. getStaticProps and getServerSideProps are dropped from client-side bundles as well: https://next-code-elimination.now.sh/

According to next-code-elimination above, the hasWindow() function DOES NOT get replaced. It stays in source and gets rendered at runtime like I would expect.

This code block is what I tested:

// This app shows what Next.js bundles for the client-side with its new SSG
// support. This editor supports TypeScript syntax.
import Cookies from 'cookies';
import Mysql from 'mysql';
import Link from 'next/link';
import SQL from 'sql-template-strings';
import Layout from '../components/Layout';

const pool = Mysql.createPool(process.env.DATABASE_URL);

const hasWindow = () => {
  return typeof window !== 'undefined';
};

export default function ({ projects }) {
  const env = hasWindow() ? 'browser' : 'server';
  return (
    <Layout>
      <h1 env={env}>Projects</h1>
      <ul>
        {projects.map((project) => (
          <li key={project.id}>
            <Link href="/projects/[id]" as={`/projects/${project.id}`}>
              <a>{project.name}</a>
            </Link>
          </li>
        ))}
      </ul>
    </Layout>
  );
}

export async function getServerSideProps({ req, res }) {
  const userId = new Cookies(req, res).get('user_id');
  const projects = await new Promise((resolve, reject) =>
    pool.query(
      SQL`SELECT id, name FROM projects WHERE user_id = ${userId};`,
      (err, results) => (err ? reject(err) : resolve(results))
    )
  );
  return { props: { projects } };
}

This is the result:

// This is the code that is bundled for the client-side:

import Link from 'next/link';
import Layout from '../components/Layout';
const hasWindow = () => {
  return typeof window !== 'undefined';
};
export var __N_SSP = true;
export default function ({ projects }) {
  const env = hasWindow() ? 'browser' : 'server';
  return (
    <Layout>
      <h1 env={env}>Projects</h1>
      <ul>
        {projects.map((project) => (
          <li key={project.id}>
            <Link href="/projects/[id]" as={`/projects/${project.id}`}>
              <a>{project.name}</a>
            </Link>
          </li>
        ))}
      </ul>
    </Layout>
  );
}

I take it all back... this makes me very happy!
😄

@dan-silk-discovery
Copy link

@Timer Will the following check work as well?

const hasWindow = () => {
  return typeof window === 'object'
}

if (!hasWindow()) {
  // server-side code
}

Or does the built-in Next optimization search specifically for when window is undefined?

@y0n3r if (!hasWindow()) { ... } won't get touched according to the next-code-elimination site (https://next-code-elimination.now.sh/)

@dan-silk-discovery
Copy link

@timneutkens thank you for the info and that link!!!

@timneutkens
Copy link
Member

next-code-elimination only shows how the elimination of getServerSideProps/getStaticProps works and doesn't include the other Babel plugin optimizations we run including the one that optimizes typeof window. In your example the code would become return true in the compiled output for the browser bundles and return false in the Node.js bundles

jose-donato added a commit to jose-donato/nikitarudenko.dev that referenced this issue Nov 12, 2020
useEffect only runs on client, i think you do not need to check for process.browser. Also, `process.browser` is deprecated in favor of `typeof window === 'undefined'` (vercel/next.js#5354 (comment))
jose-donato added a commit to jose-donato/nikitarudenko.dev that referenced this issue Nov 12, 2020
useEffect only runs on client, i think you do not need to check for process.browser. Also, `process.browser` is deprecated in favor of `typeof window === 'undefined'` (vercel/next.js#5354 (comment))
@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests