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

Remix V1 to V2: Error console logs a weird issue but everything works fine #7593

Closed
1 task done
AlemTuzlak opened this issue Oct 5, 2023 · 10 comments · Fixed by #7745
Closed
1 task done

Remix V1 to V2: Error console logs a weird issue but everything works fine #7593

AlemTuzlak opened this issue Oct 5, 2023 · 10 comments · Fixed by #7745
Assignees
Labels
bug Something isn't working renderer:react

Comments

@AlemTuzlak
Copy link
Contributor

AlemTuzlak commented Oct 5, 2023

What version of Remix are you using?

v2.0.1

Are all your remix dependencies & dev-dependencies using the same version?

  • Yes

Steps to Reproduce

My entry.server.ts:

export default async function handleRequest(
  request: Request,
  responseStatusCode: number,
  responseHeaders: Headers,
  remixContext: EntryContext
) { 
  const instance = createInstance(); 
  const lng = await i18n.getLocale(request); 
  let ns = i18n.getRouteNamespaces(remixContext);
 
  await instance
    .use(initReactI18next) 
    .init({
      ...i18nextOptions,  
      lng,
      ns,  
      resources 
    });
 
  instance.services.formatter?.add("lowercase", value => value.toLowerCase());

  const ABORT_DELAY = 10000; 
  let markup = <RemixServer abortDelay={ABORT_DELAY} context={remixContext} url={request.url} />;

  return new Promise((resolve, reject) => {
    let shellRendered = false;

    const { pipe, abort } = renderToPipeableStream(markup, {
      onAllReady() {
        let body = new PassThrough();
        const stream = createReadableStreamFromReadable(body);
        responseHeaders.set("Content-Type", "text/html");

        resolve(
          new Response(stream, {
            status: responseStatusCode,
            headers: responseHeaders
          })
        );
        pipe(body);
      },
      onShellError(err: unknown) {
        reject(err);
      },
      onError(error: unknown) {
        responseStatusCode = 500;
        // Log streaming rendering errors from inside the shell.  Don't log
        // errors encountered during initial shell rendering since they'll
        // reject and get logged in handleDocumentRequest.
        if (shellRendered) {
          console.error(error);
        }
      }
    });
    setTimeout(abort, ABORT_DELAY);
  });
}

My remix.config.js:

const { createRoutesFromFolders } = require("@remix-run/v1-route-convention");
const ignoredRoutePatterns = ["**/.*", "**/components/**", "**/integration/*.test.*"];

/** @type {import('@remix-run/dev').AppConfig} */
module.exports = { 
  ignoredRouteFiles: ignoredRoutePatterns,
  serverDependenciesToBundle: [/^marked.*/],
  tailwind: true,
  serverModuleFormat: "cjs", 
  watchPaths: ["./public/locales/en/common.json", "./public/locales/fr/common.json"],
  routes(defineRoutes) {
    return createRoutesFromFolders(defineRoutes, {
      //Our convention is to not consider anything under the routes/**/components to be a route.
      ignoredFilePatterns: ignoredRoutePatterns
    });
  }
};

my Remix dev server command:

 remix dev 

Expected Behavior

There is no error in the console, everything works fine.

Actual Behavior

So a little context, we have a 1.19.3 app running on cjs. After upgrading it to v2 everything is working fine BUT we keep getting this error whenever we start the dev server OR hard reload the page, between navigations it works fine. Any idea what might be causing this?
I run the dev server, everything works fine but I get the following error:

Warning: React.createElement: type is invalid -- expected a string (for built-in components) or a class/function (for composite components) but got: object. You likely forgot to export your component from the file it's defined in, or you might have mixed up default and named imports.
    at RemixServer (C:\Users\Alem Tuzlak\projects\MAPflow\node_modules\@remix-run\react\dist\server.js:47:3)
@AlemTuzlak
Copy link
Contributor Author

I've tried the following:

  • Remove all routes that do not export a default component (resource routes) didn't help
  • Remove the i18n integration completely, didn't work
  • Removed entry.server.ts from the project, didn't work
  • Console logged the server.js createElement types, the error boundary is a class, the StaticRouterProvider is a function and RemixContext.provider is a context provider so this seems okay (not sure if the fact that it's not a function/class can be tripping up createElement but it shouldn't?)
  • Deleted everything from my root and exported an empty document, didn't work

@theskillwithin
Copy link

I am getting this same error on hot reloading randomly

@brophdawg11
Copy link
Contributor

Can anyone provide a reproduction repo or stackblitz of this issue?

@brophdawg11 brophdawg11 added needs-response We need a response from the original author about this issue/PR renderer:react labels Oct 12, 2023
@nmackey
Copy link

nmackey commented Oct 18, 2023

I'm also seeing this error during HMR after upgrading from 1.19.3 to 2.1.0

@github-actions github-actions bot removed the needs-response We need a response from the original author about this issue/PR label Oct 18, 2023
@AlemTuzlak
Copy link
Contributor Author

@brophdawg11 After a bit of experimentation it seems that after removing:

 routes(defineRoutes) {
    return createRoutesFromFolders(defineRoutes, {
      //Our convention is to not consider anything under the routes/**/components to be a route.
      ignoredFilePatterns: ignoredRoutePatterns
    });
  }

from my remix.config.js file the error goes away.

After an investigation I have figured out the root cause of this, at least for my case, before version 2 we had a route that had exports before but we commented out everything so the route didn't export anything anymore, you can find the exact reproduction here:
https://stackblitz.com/edit/remix-run-remix-5xyzrm?file=app%2Froutes%2Fno-export%2Findex.tsx

This worked in v1 without console logging an error but now it logs the reported error above, I am not sure if this is expected behavior or not, after commenting back in an export of a loader it goes away.

@nmackey
Copy link

nmackey commented Oct 19, 2023

I'm using https://github.com/kiliman/remix-flat-routes which requires ignoring all regular route files and using the routes function in remix.config.js as well so it could definitely be related to that configuration. I'll experiment some today and report back.

@brophdawg11
Copy link
Contributor

Ahh, ok thank you for tracking that down @AlemTuzlak. We ran into something like this before and looks like we need the same fix in another spot or two. The default export when not defined ends up being {}, and if not caught and we send that as the Component in the React Router layer we try to call createElement({}). This is something we should probably look into at the compiler level but it may also not be an issue with vite so may not be worth the investigation since it's such an easy catch/fix at a higher level.

We have this check for lazily loaded routes to avoid assigning the default {} as the route Component. We probably need that same check here and here to cover all of the paths.

If anyone wants to take a stab at this it should be something we can assert via the console in a playwright test and a pretty simple fix. Otherwise I'll get a fix in once I wrap up some current stuff I'm focused on.

@brophdawg11 brophdawg11 self-assigned this Oct 19, 2023
@kiliman
Copy link
Collaborator

kiliman commented Oct 19, 2023

I'm using https://github.com/kiliman/remix-flat-routes ...

@nmackey Definitely tag me if you think this is a bug in remix-flat-routes, or better yet file an issue. Thanks!

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 2.2.0-pre.0 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

Copy link
Contributor

🤖 Hello there,

We just published version 2.2.0 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working renderer:react
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants