Recoil icon indicating copy to clipboard operation
Recoil copied to clipboard

[SSR][NextJS] Duplicate atom key during development and during production build in nextjs

Open janus-reith opened this issue 3 years ago • 41 comments

Both during local development and also when building a production build, I’m getting a duplicate atom key for each key and each page using it after the first one.

I put together a quick sandbox to demonstrate the issue: https://codesandbox.io/s/flamboyant-sea-tqlky

The errors show up in the build log, a quick way to test is using the integrated vercel deployment button. Looking at the default nextjs example, I can‘t spot any special settings there that would prevent whatever duplication is going on from happening: https://github.com/vercel/next.js/tree/canary/examples/with-recoil That example however only makes use of recoil state on one page.

janus-reith avatar Nov 11 '20 23:11 janus-reith

Also mentioned in:
https://github.com/facebookexperimental/Recoil/issues/213#issuecomment-659349900

This is how it looks like in the Vercel deployment build: image

alexilyaev avatar Nov 12 '20 11:11 alexilyaev

Has this been solved? I'm having the same issue.

leonardomgt avatar Nov 16 '20 16:11 leonardomgt

@leonardomgt I was not able to solve this yet.

@drarmstr Tagging you as you seem to be a main contributor to this project, any chance you could have look at this? Thanks!

janus-reith avatar Nov 16 '20 17:11 janus-reith

@janus-reith - Sorry, I'm not very familiar with Next.JS or SSR.

drarmstr avatar Nov 17 '20 19:11 drarmstr

@janus-reith - Sorry, I'm not very familiar with Next.JS or SSR.

Next.js has a concept of Pages. Imagine an SPA with multiple entry points, like an Admin and a Client App, both importing a module that declares a Recoil atom. So it doesn't have to be Next.js related.
Essentially it's a single Node.js process that executes JS files (build process) that eventually declare the same atom several times.

In development, when a file is changed, Next.js re-builds the relevant page entry file.
Because it's the same Node.js process, the atom has already been declared.
The same thing can happen with HMR when the file change triggers a rebuild of the whole file, or even when the atom is declared inside a component lifecycle/hook and only that is being hot-replaced.

Basically, I can't think of a solution to this problem besides providing a configuration setting to disable this check/warning.
At a minimum, developers should be able to disable it.
In Next.js or SSR that would be when running on the server.
In HMR, that would be when not in production env.

A zero config alternative might be tempting. E.g. relying on NODE_ENV=production like React does for it's optimizations.
It would probably solve the HMR issue, but not the build of Next.js, as that runs with NODE_ENV=production obviously.
Although, that would already be better than what we currently have.

alexilyaev avatar Nov 17 '20 22:11 alexilyaev

Related to #247

drarmstr avatar Nov 18 '20 22:11 drarmstr

Thanks @drarmstr for the response nonetheless, and thanks @alexilyaev for the detailed description.

Regarding the multiple entry points, I wonder if this could be worked around by some more advanced tracing logic to distinguish actual redeclaration of the key from the same code being executed again using the callsite? I guess that depending on the way atoms are initialized there could be cases where this is not fully reliable(e.g. some dynamic declaration with variable-based key names ), also I'm not sure about browser support but still, maybe that is a feasible approach to consider.

janus-reith avatar Nov 22 '20 00:11 janus-reith

Me to for this issue.

Him-2C avatar Dec 09 '20 15:12 Him-2C

Hey guys, any update on this issue? Has anybody come up with a workaround? Thanks!

juanpprieto avatar Feb 26 '21 21:02 juanpprieto

Ok guys, here's a temporary "solution" via a webpack "plugin" that kills the Duplicate Warnings by literally modifying the recoil source. sadness

from...

function registerNode(node) {
  if (nodes.has(node.key)) {
    ....code
    console.warn(message); // @oss-only
  }
  ....code
}

to:

function registerNode(node) {
  if (nodes.has(node.key)) {
    ....code
    (!process.env.__NEXT_PROCESSED_ENV && !process.env.SILENCE_RECOIL_DUPE_WARNING) && console.warn(message); // @oss-only
  }
  ....code
}

✅ Am I desperate? ✅ Is this ugly? ✅ is it a huge hack? ✅ Works? tested in next@^10.0.3 / recoil@^0.1.2 with SILENCE_RECOIL_DUPE_WARNING=true ✅ Could work outside nextjs?. Maybe.. SILENCE_RECOIL_DUPE_WARNING=true ✅ Works in netlify ? ⚠️ Is this a joke? 🚫 Fully tested (not even close) 🤷‍♂️ Its safe to use in production

// feel free to replace these
const fs = require('promise-fs')
const path = require('path')

/**
 Webpack plugin that modifies /node_modules/recoil/[...]/recoil.js
 adding process.env conditionals before the registerNode/duplicate key warning log
**/
{
  apply: (compiler) => {
    return compiler.hooks.run.tapAsync(
      "SilenceRecoilDupeWarningBeforeCompile",
      async (compilation, callback) => {
        const SILENCING_VARS = [
          "__NEXT_PROCESSED_ENV",
          "SILENCE_RECOIL_DUPE_WARNING",
        ];

        const silenceModule = async (module) => {
          // target recoil module & path (Adjust for your own project) 👈 mono repo setup
          const recoilModulePath = path.join(
            process.cwd(),
            "..",
            `/node_modules/${module}`
          );

          console.log(`Disabling ${recoilModulePath} warnings`);

          // read the source module
          const recoilSource = await fs.readFile(recoilModulePath, "utf8");

          // any of these will silence the warning
          const conditionalString = `(${SILENCING_VARS.reduce(
            (condStr, envVar) => {
              condStr = [...condStr, `!process.env.${envVar}`];
              return condStr;
            },
            []
          ).join(" && ")}) &&`;

          // regex and replace function
          const duplicateWarningRegex = /(if \(nodes\.has\(node\.key\)\) \{.*?)(console\.warn\(message\)\;)(.*?\})/gs;

          const duplicateWarningReplace = (str, $1, $2, $3) => {
            if ($1 && $3) {
              return `${
                $1 ? $1.replace(conditionalString, "") : ""
              }${conditionalString} console.warn(message);${$3}`;
            }
            return str;
          };

          // modify the recoil source file
          const modifiedRecoilSource = recoilSource.replace(
            duplicateWarningRegex,
            duplicateWarningReplace
          );

          // overwrite the recoil module - cause you can 🤷‍♂️
          await fs.writeFile(recoilModulePath, modifiedRecoilSource, "utf8");
        };

        try {
          const TARGET_RECOIL_MODULES = [
            "recoil/cjs/recoil.js",
            "recoil/es/recoil.js",
            "recoil/umd/recoil.js",
          ];

          for (let m = 0; m < TARGET_RECOIL_MODULES.length; m++) {
            const currentModule = TARGET_RECOIL_MODULES[m];
            await silenceModule(currentModule);
          }

          console.log("Disabled recoil duplicate key warning (hopefully).");
          callback();
        } catch (error) {
          console.log("SilenceRecoilDupeWarningBeforeCompile", error);
          callback();
        }
      }
    );
  };
}

Hopefully this enthusiasm sparks a proper solution... 👍

UPDATE: less invasive solution for nextjs here

juanpprieto avatar Apr 05 '21 16:04 juanpprieto

I am reproducing this on my Next.js app. I only do SSR when running locally in dev mode, otherwise in production - I export static files via SSG. From what I can tell, this is just a warning and safe to ignore. Even though I'm a newbie to Recoil - it seems to be working 😨? I actually only see the output in the server logs... not the browser. Although the output is annoying, I just want to make sure this isn't a blocker in using Recoil. Can I just ignore the output? Or is this issue about actual broken functionality?

adamhenson avatar Jul 17 '21 14:07 adamhenson

I am reproducing this on my Next.js app. I only do SSR when running locally in dev mode, otherwise in production - I export static files via SSG. From what I can tell, this is just a warning and safe to ignore. Even though I'm a newbie to Recoil - it seems to be working 😨? I actually only see the output in the server logs... not the browser. Although the output is annoying, I just want to make sure this isn't a blocker in using Recoil. Can I just ignore the output? Or is this issue about actual broken functionality?

You can ignore the warning output (if you can bare it), functionality is not affected (We have a few sites in production working just fine) :(

juanpprieto avatar Jul 19 '21 15:07 juanpprieto

I'm very eager to start using Recoil in my applications, but errors like this make me wary of adopting. Any official update on this? I'd prefer a solution that doesn't involve modifying the source code in node_modules.

R-Bower avatar Sep 16 '21 18:09 R-Bower

I feel you, I love recoil too much (specially on Nextjs), so I've taken a leap of faith that the staff will do something about it since SSR environments are here to stay and we have very complex production apps running beautifully with it. Fingers crossed 🙏🏼

juanpprieto avatar Sep 16 '21 18:09 juanpprieto

@juanpprieto I've found a less invasive method that relies on the intercept-stdout package.

Add the following to next.config.js (outside of the exported configuration):

const intercept = require("intercept-stdout")

// safely ignore recoil warning messages in dev (triggered by HMR)
function interceptStdout(text) {
  if (text.includes("Duplicate atom key")) {
    return ""
  }
  return text
}

if (process.env.NODE_ENV === "development") {
  intercept(interceptStdout)
}

This way I don't have to modify the source code in node_modules. Do you see any downsides to this approach?

R-Bower avatar Sep 21 '21 01:09 R-Bower

@juanpprieto I've found a less invasive method that relies on the intercept-stdout package.

Add the following to next.config.js (outside of the exported configuration):

const intercept = require("intercept-stdout")

// safely ignore recoil warning messages in dev (triggered by HMR)
function interceptStdout(text) {
  if (text.includes("Duplicate atom key")) {
    return ""
  }
  return text
}

if (process.env.NODE_ENV === "development") {
  intercept(interceptStdout)
}

This way I don't have to modify the source code in node_modules. Do you see any downsides to this approach?

Hey @R-Bower!

On paper this seems 1000 times less invasive than my hack!

I will test at my end and let you know! — Thanks for the heads up!

juanpprieto avatar Sep 22 '21 00:09 juanpprieto

The error still shows on in-browser hot reloads. Not sure if there's a way to suppress those other than the chrome console regex filter.

R-Bower avatar Sep 22 '21 00:09 R-Bower

The error still shows on in-browser hot reloads. Not sure if there's a way to suppress those other than the chrome console regex filter.

Yes, intercept-stdout works well for stdout suppressing in both development and production. Unfortunately, browser warnings are still there. I tried using webpack native stats suppressing and webpack-filter-warnings-plugin, but those two fail to mute recoil warning for some reason, even though it does block other warnings I wanted to mute such as Critical dependency: the request of a dependency is an expression.

This why I ended up doing the hack I did unfortunately. If anyone knows another option to try for muting browser warnings I'd love to hear and try them. Thanks!

juanpprieto avatar Sep 22 '21 15:09 juanpprieto

@R-Bower think i may have found a cleaner way to kill recoil browser warnings.

src/pages/_app.jsx — Intercept client/browser warnings. Outside the exported react component

const memoize = (fn) => {
  let cache = {};
  return (...args) => {
    let n = args[0];
    if (n in cache) {
      return cache[n];
    }
    else {
      let result = fn(n);
      cache[n] = result;
      return result;
    }
  }
}


// ignore in-browser next/js recoil warnings until its fixed.
const mutedConsole = memoize((console) => ({
  ...console,
  warn: (...args) => args[0].includes('Duplicate atom key')
    ? null
    : console.warn(...args)
}))

global.console = mutedConsole(global.console);

next.config.js — Intercept stdout warnings. Outside the exported webpack config

const intercept = require("intercept-stdout")

// safely ignore recoil stdout warning messages 
function interceptStdout(text) {
  if (text.includes('Duplicate atom key')) {
    return ''
  }
  return text
}

// Intercept in dev and prod
intercept(interceptStdout)

Let me know if this works for you!

juanpprieto avatar Sep 22 '21 16:09 juanpprieto

@juanpprieto It works! Thanks :)

hsh2001 avatar Sep 28 '21 07:09 hsh2001

@R-Bower think i may have found a cleaner way to kill recoil browser warnings.

src/pages/_app.jsx — Intercept client/browser warnings. Outside the exported react component

// ignore in-browser next/js recoil warnings until its fixed.
const mutedConsole = memoize((console) => ({
  ...console,
  warn: (...args) => args[0].includes('Duplicate atom key')
    ? null
    : console.warn(...args)
}))
global.console = mutedConsole(global.console);

next.config.js — Intercept stdout warnings. Outside the exported webpack config

const intercept = require("intercept-stdout")

// safely ignore recoil stdout warning messages 
function interceptStdout(text) {
  if (text.includes('Duplicate atom key')) {
    return ''
  }
  return text
}

// Intercept in dev and prod
intercept(interceptStdout)

Let me know if this works for you!

This hangs when I start the server on next.js

spookyvert avatar Oct 02 '21 03:10 spookyvert

Duplicate atom key "userAtomState". This is a FATAL ERROR in production. But it is safe to ignore this warning if it occurred because of hot module replacement.

Nabin0433 avatar Dec 06 '21 16:12 Nabin0433

@R-Bower think i may have found a cleaner way to kill recoil browser warnings. src/pages/_app.jsx — Intercept client/browser warnings. Outside the exported react component

// ignore in-browser next/js recoil warnings until its fixed.
const mutedConsole = memoize((console) => ({
  ...console,
  warn: (...args) => args[0].includes('Duplicate atom key')
    ? null
    : console.warn(...args)
}))
global.console = mutedConsole(global.console);

next.config.js — Intercept stdout warnings. Outside the exported webpack config

const intercept = require("intercept-stdout")

// safely ignore recoil stdout warning messages 
function interceptStdout(text) {
  if (text.includes('Duplicate atom key')) {
    return ''
  }
  return text
}

// Intercept in dev and prod
intercept(interceptStdout)

Let me know if this works for you!

This hangs when I start the server on next.js

Me too. In terminal will show compiled successfully, but browser will always loading.

TofuTseng avatar Dec 07 '21 08:12 TofuTseng

Any news about this ?

Here my temporary small fix :

// next.config.js
const intercept = require('intercept-stdout');

intercept((text) => (text.includes('Duplicate atom key') ? '' : text));

johackim avatar Jan 22 '22 15:01 johackim

I published next-intercept-stdout that wraps intercept-stdout as the form of the Next.js plugin.

  1. I didn't want to call a function separate from other NextConfig values and plugins. It makes the action look suspicious and less coupled.
  2. In addition, intercept-stdout was last distributed about six years ago, so it seemed necessary to cover the implementation separately in case it no longer works.

https://github.com/junhoyeo/next-intercept-stdout

// Example for Recoil
// next.config.js
const withInterceptStdout = require('next-intercept-stdout');
const withSvgr = require('@newhighsco/next-plugin-svgr');

module.exports = withInterceptStdout(
  withSvgr({
    reactStrictMode: true,
    svgrOptions: {},
  }),
  (text) => (text.includes('Duplicate atom key') ? '' : text),
);

junhoyeo avatar Jan 27 '22 07:01 junhoyeo

Not for Next.js, I'm in react with Node.js. In my case, I solved this issue by using a pacakge called uuid, which generates random numbers in Node.js. I think having the same key in recoil doesn't mean having the same recoil state value. Each time re-rendering occurs, the recoil state value changes, and a different key is required each time.

You can use it as follows

import { atom } from 'recoil';
import { v1 } from 'uuid';

function Fname(name) {
  return atom({
    key: `${name}/${v1()}`,
    default: '',
  });
}

export { Fname };

Additionally I got a name from each component using recoil value. If unnecessary, ${v1()} will be ok.

keinn51 avatar Feb 25 '22 08:02 keinn51

It seem like this would be addressed by now. Getting this in Stackblitz and other environments. Not using Next.js.

ryanwellsdotcom avatar Mar 06 '22 02:03 ryanwellsdotcom

Yeah this is not only due to Next.js's SSR. I am currently implementing CSR with Next.js and still shows the duplication message. I dont think this is Next.js's issue at all.

This only happens during dev environment though. Although not causing any issue, it is the message for us to freak out.

tpatalas avatar Mar 06 '22 02:03 tpatalas

@R-Bower think i may have found a cleaner way to kill recoil browser warnings. src/pages/_app.jsx — Intercept client/browser warnings. Outside the exported react component

// ignore in-browser next/js recoil warnings until its fixed.
const mutedConsole = memoize((console) => ({
  ...console,
  warn: (...args) => args[0].includes('Duplicate atom key')
    ? null
    : console.warn(...args)
}))
global.console = mutedConsole(global.console);

next.config.js — Intercept stdout warnings. Outside the exported webpack config

const intercept = require("intercept-stdout")

// safely ignore recoil stdout warning messages 
function interceptStdout(text) {
  if (text.includes('Duplicate atom key')) {
    return ''
  }
  return text
}

// Intercept in dev and prod
intercept(interceptStdout)

Let me know if this works for you!

This hangs when I start the server on next.js

Worked wonderfully, thanks!

royz avatar Apr 03 '22 15:04 royz

Updated working version for "recoil": "^0.7.2",

Tested and working on @remix

app/entry.server.jsx

import intercept from 'intercept-stdout';

// Silence Recoil duplicate atom errors (stdout/server)
function muteStdout(log) {
  if (log.includes('Expectation Violation:')) {
    return '';
  }
  return log;
}

intercept(muteStdout, muteStdout);

app/entry.client.jsx

const memoize = (fn) => {
  let cache = {};
  return (...args) => {
    let n = args[0];
    if (n in cache) {
      return cache[n];
    }
    else {
      let result = fn(n);
      cache[n] = result;
      return result;
    }
  }
}

// Silence Recoil duplicate atom errors (client/browser)
const mutedConsole = memoize((console) => ({
  ...console,
  error: (...args) => {
    return args[0]?.name === 'Expectation Violation'
      ? null
      : console.error(...args)
  }
}))

// Overload before hydrate 
window.console = mutedConsole(window.console)

hydrate(<RemixBrowser />, document);

juanpprieto avatar Apr 28 '22 22:04 juanpprieto