🧩 Plasmo Developers•2y ago

react-router HMR Issues when lazy-load

Is HMR working correctly for everyone? I noticed that it's no longer working for me and I have to manually refresh the page each time. The only major change I did was switch to the new React Router 6.4 system: https://reactrouter.com/en/main so it might be that

Home v6.11.2

78 Replies

Arcane•2y ago

@nahtnam has reached level 10. GG!

lab•2y ago

I have seen someone reporting issue about this before as well re: latest react-router Does this happen only when you change a sub-page, or even when you change the root entry file, HMR won't hit at all? @nahtnam I assume this is happening in a newtab page right?

nahtnamOP•2y ago

Correct on the newtab let me check if updating the root file HMRs HMR works if I edit the root file (outside of the react router provider)

lab•2y ago

that's tricky... I wonder if this is because of provider and caching :-?.. like if they put the Element inside a memo and cache it

nahtnamOP•2y ago

https://github.com/remix-run/react-router/issues/9493 Not a very helpful response

lab•2y ago

maybe HMR couldn't propagate due to their new provider cached the view, and HMR is blocked from applying view change due to memo cache (?)) I'd investigate this: https://reactrouter.com/en/main/route/should-revalidate

nahtnamOP•2y ago

Oh you know what, one other thing I lazy load every route

lab•2y ago

if vite's having the same issue, then anyone using the standard react-dev-runtime would have the same issue I recon hmmm try disabling lazyload for dev bundle using process.env.NODE_ENV When using lazyload and create prod bundle, does it spit out multiple file or still a huge newtab.js bundle?

nahtnamOP•2y ago

Let me try that, it returns a bunch of files, one for each page

lab•2y ago

If it spits out a huge newtab bundle (which I kinda expect it to atm - we don't explicitly split bundle yet due to extension import protocol still need to be investigated), I don't think lazy load would help much. But if it does spit out one for each page (surprise, it just work??), then I'd keep lazy for prod but remove it for dev and see if HMR work

nahtnamOP•2y ago

Yep, youre right That was the issue, changing it to not lazy load the solution However I'm not entirely sure how to only lazy load in prod, let me look into it

export const settingsPage = exportPage({
  path: '/settings',
  layout: <LayoutWithNavigation exitPath={indexPath} isModal />,
  // lazy: () => import('./page'),
  // element: <Component />,
});

export const settingsPage = exportPage({
  path: '/settings',
  layout: <LayoutWithNavigation exitPath={indexPath} isModal />,
  // lazy: () => import('./page'),
  // element: <Component />,
});

I wonder if I import but don't use <Component if it will be tree shaken out in production

lab•2y ago

yes it will, there's a technique for this as well I think the treeshaker will actually prune ternary as well so like:

export const settingsPage = process.env.NODE_ENV === "development" ? exportPage({
  path: '/settings',
  layout: <LayoutWithNavigation exitPath={indexPath} isModal />,
  // lazy: () => import('./page'),
  // element: <Component />,
}): exportPage({
  path: '/settings',
  lazy: () => import('./page'),
  element: <Component />,
});

export const settingsPage = process.env.NODE_ENV === "development" ? exportPage({
  path: '/settings',
  layout: <LayoutWithNavigation exitPath={indexPath} isModal />,
  // lazy: () => import('./page'),
  // element: <Component />,
}): exportPage({
  path: '/settings',
  lazy: () => import('./page'),
  element: <Component />,
});

nahtnamOP•2y ago

🔥 Let me give that a shot, ill get back to you in an hour

lab•2y ago

Now I'm not sure if you can set that process.env into a variable tho.. since the static evaluator might not dig deeper into the runtime stack so you might need to be verbose with this one, OR have a giant if statement at the top

nahtnamOP•2y ago

Alright so this works:

import * as page from './page';

export const settingsPage = exportPage({
  path: '/settings',
  layout: <LayoutWithNavigation exitPath={indexPath} isModal />,
  lazy: () => {
    if (process.env.NODE_ENV === 'production') return import('./page');

    return Promise.resolve(page);
  },
});

import * as page from './page';

export const settingsPage = exportPage({
  path: '/settings',
  layout: <LayoutWithNavigation exitPath={indexPath} isModal />,
  lazy: () => {
    if (process.env.NODE_ENV === 'production') return import('./page');

    return Promise.resolve(page);
  },
});

in development but in production it fails. I also tried variations of the code above. I think the issue seems to be that you can't import('./page') actually have the import of page in the same file

🟡 0      | @plasmohq/parcel-bundler
🔴 ERROR  | Got unexpected undefined
🟡 11     | Error: Got unexpected undefined
    at nullthrows (/Users/nahtnam/Desktop/code.nosync/ludicrous/caddie-dash/node_modules/nullthrows/nullthrows.js:7:15)
    at ye (/Users/nahtnam/Desktop/code.nosync/ludicrous/caddie-dash/node_modules/@plasmohq/parcel-bundler/dist/index.js:1:9569)
    at Object.bundle (/Users/nahtnam/Desktop/code.nosync/ludicrous/caddie-dash/node_modules/@plasmohq/parcel-bundler/dist/index.js:1:16538)
    at BundlerRunner.bundle (/Users/nahtnam/Desktop/code.nosync/ludicrous/caddie-dash/node_modules/@parcel/core/lib/requests/BundleGraphRequest.js:288:23)
    at async Object.run (/Users/nahtnam/Desktop/code.nosync/ludicrous/caddie-dash/node_modules/@parcel/core/lib/requests/BundleGraphRequest.js:156:17)
    at async RequestTracker.runRequest (/Users/nahtnam/Desktop/code.nosync/ludicrous/caddie-dash/node_modules/@parcel/core/lib/RequestTracker.js:756:20)
    at async Object.run (/Users/nahtnam/Desktop/code.nosync/ludicrous/caddie-dash/node_modules/@parcel/core/lib/requests/ParcelBuildRequest.js:56:7)
    at async RequestTracker.runRequest (/Users/nahtnam/Desktop/code.nosync/ludicrous/caddie-dash/node_modules/@parcel/core/lib/RequestTracker.js:756:20)
    at async D._build (/Users/nahtnam/Desktop/code.nosync/ludicrous/caddie-dash/node_modules/@plasmohq/parcel-core/dist/index.js:1:7089)
    at async D.run (/Users/nahtnam/Desktop/code.nosync/ludicrous/caddie-dash/node_modules/@plasmohq/parcel-core/dist/index.js:1:5729)

🟡 0      | @plasmohq/parcel-bundler
🔴 ERROR  | Got unexpected undefined
🟡 11     | Error: Got unexpected undefined
    at nullthrows (/Users/nahtnam/Desktop/code.nosync/ludicrous/caddie-dash/node_modules/nullthrows/nullthrows.js:7:15)
    at ye (/Users/nahtnam/Desktop/code.nosync/ludicrous/caddie-dash/node_modules/@plasmohq/parcel-bundler/dist/index.js:1:9569)
    at Object.bundle (/Users/nahtnam/Desktop/code.nosync/ludicrous/caddie-dash/node_modules/@plasmohq/parcel-bundler/dist/index.js:1:16538)
    at BundlerRunner.bundle (/Users/nahtnam/Desktop/code.nosync/ludicrous/caddie-dash/node_modules/@parcel/core/lib/requests/BundleGraphRequest.js:288:23)
    at async Object.run (/Users/nahtnam/Desktop/code.nosync/ludicrous/caddie-dash/node_modules/@parcel/core/lib/requests/BundleGraphRequest.js:156:17)
    at async RequestTracker.runRequest (/Users/nahtnam/Desktop/code.nosync/ludicrous/caddie-dash/node_modules/@parcel/core/lib/RequestTracker.js:756:20)
    at async Object.run (/Users/nahtnam/Desktop/code.nosync/ludicrous/caddie-dash/node_modules/@parcel/core/lib/requests/ParcelBuildRequest.js:56:7)
    at async RequestTracker.runRequest (/Users/nahtnam/Desktop/code.nosync/ludicrous/caddie-dash/node_modules/@parcel/core/lib/RequestTracker.js:756:20)
    at async D._build (/Users/nahtnam/Desktop/code.nosync/ludicrous/caddie-dash/node_modules/@plasmohq/parcel-core/dist/index.js:1:7089)
    at async D.run (/Users/nahtnam/Desktop/code.nosync/ludicrous/caddie-dash/node_modules/@plasmohq/parcel-core/dist/index.js:1:5729)

lab•2y ago

yep I think it would cause some type of circular injection does the HMR work in dev mode with that?

nahtnamOP•2y ago

Yes it does So just need it to build

lab•2y ago

@nahtnam just released 0.71.1, try it with hoist and for your remote import, use the suggestion in the other thread

nahtnamOP•2y ago

Alright so it builds without hoist, so thats good news Sorry wrong thread Okay so the 0.71.0 issue is resolved, hoist doesnt work. With the solution you mentioned in the other thread (adding _) it throws an error:

🔴 ERROR  | 
  × cannot reassign to a variable declared with `const`
       ╭─[20432:1]
 20432 │ });
 20433 │ parcelRequire.register("hzY9l", function(module, exports) {
 20434 │ 
 20435 │ $parcel$export(module.exports, "resolveFetch", () => $ccc45d025041ee02$export$98d92b1aa79f8cc7, (v) => $ccc45d025041ee02$export$98d92b1aa79f8cc7 = v);
       ·                                                                                                        ────────────────────┬────────────────────
       ·                                                                                                                            ╰── cannot reassign

🔴 ERROR  | 
  × cannot reassign to a variable declared with `const`
       ╭─[20432:1]
 20432 │ });
 20433 │ parcelRequire.register("hzY9l", function(module, exports) {
 20434 │ 
 20435 │ $parcel$export(module.exports, "resolveFetch", () => $ccc45d025041ee02$export$98d92b1aa79f8cc7, (v) => $ccc45d025041ee02$export$98d92b1aa79f8cc7 = v);
       ·                                                                                                        ────────────────────┬────────────────────
       ·                                                                                                                            ╰── cannot reassign

lab•2y ago

Yeah so rn this is focusing on that remote code import right I'm seeing a similar issue as well, trying to see if I can bundle it somehow :d....

nahtnamOP•2y ago

Gotcha, not that big of a deal to not use hoist, some extra code gets bundled Im more annoyed by the lazy import thing 😂 trying to play around with it, for sure id like to use lazy because the excess code being loaded slows the newtab down and im trying to keep it as light as possible

lab•2y ago

@nahtnam try out 0.71.2 for this, can the if statement be moved one level up? :-?

nahtnamOP•2y ago

Hmmm

🔴 ERROR | EACCES: permission denied, open '/Users/nahtnam/Desktop/code.nosync/ludicrous/caddie-dash/.plasmo/cache/remote-code/240c7a708a5899b1.js'

with and without hoist Tried the solution you posted as well process.env.NODE_ENV === "development" ? exportPage(..) : exportPage(...), I think the issue is just having both imports in the same file

lab•2y ago

oh wat

nahtnamOP•2y ago

Fails in CI with the same error too

lab•2y ago

do you have the import in multiple file?

nahtnamOP•2y ago

Nope, I have two imports though

lab•2y ago

hmmm... BTW I got a solution for your original problem (prod vs dev router) potential solution xd So the idea is that you would have a popup.tsx and a popup.production.tsx And in the popup.production, you would specify lazy loading etc... @nahtnam ^ do you think that direction might solve the problem?

nahtnamOP•2y ago

I dont think that would work with the way that i've laid out the code 😦 Each of the route.tsx export a route, so I think id have to duplicate all of those

lab•2y ago

Does HMR work if you do lazy but with constant invalidation lol Like this:

 <Route
      path="/lazy"
      lazy={() => import("./lazy-route")}
      shouldRevalidate={() => {
        return true
      }}
    />

 <Route
      path="/lazy"
      lazy={() => import("./lazy-route")}
      shouldRevalidate={() => {
        return true
      }}
    />

nahtnamOP•2y ago

Nope, I put a console.log in there too and its not firing so I assume shouldRevalidate isnt run even with theres an HMR update

lab•2y ago

hmm if that's the case

lab•2y ago

@nahtnam here's my solution: https://github.com/PlasmoHQ/examples/blob/main/with-react-router/routes/index.tsx

GitHub

examples/index.tsx at main · PlasmoHQ/examples

🔰 Example projects that demonstrate how to use the Plasmo Framework and integrate with popular tools - examples/index.tsx at main · PlasmoHQ/examples

lab•2y ago

also just published a fix for this in 0.71.3! basically, the lazy-router is a thin wrapper around the demoview (that's going to be lazy loaded), then if I want to render directly for HMR, the dev version consume the view directly hmm... just inspected the bundle... the lazy view is still bundled in the top bundle xd.....

nahtnamOP•2y ago

😛 Also build without hoist works in 0.71.3 but with hoist it's still broken nvm I actually dont think its the import http stuff, its broken even if I comment those out So weird: https://gist.github.com/nahtnam/67798f330ae629b62220e51701f0fd09 I was speculating that maybe if with --hoist it's more aggressive, maybe the lazy import thing would work

lab•2y ago

dang it

Arcane•2y ago

@louis has reached level 46. GG!

lab•2y ago

I couldn't figre out a good way to ensure both lazy loading and also HMR in dev on the same file lol unless we provide a custom lazy importer similar to nextjs's dynamic thingy that's basically what they did also iirc, nextjs dynamic trigger a reload instead of HMR lol xD....

nahtnamOP•2y ago

Hmm no worries, I might just undo the lazys in that case

lab•2y ago

Is there any place that a const is being reassigned in your code somehow :d.... weird that it's... bundled ok in dev tho lol but that error is likely when it try to optimize the code :-?..

nahtnamOP•2y ago

One last question, I dont fully understand how import works but do you know if its possible to have top level dynamic imports? One potential direction could be:

const routes = [
  { path: '/', location: './page' },
  { path: '/settings', location: './settings/page' }
]

const router = process.env.NODE_ENV === 'production' ? routes.map(route => ({
  path: route.path
  lazy: () => import(route.location)
})) : routes.map(route => ({
  path: route.path
  Component: import(route.location).Component
}))

const routes = [
  { path: '/', location: './page' },
  { path: '/settings', location: './settings/page' }
]

const router = process.env.NODE_ENV === 'production' ? routes.map(route => ({
  path: route.path
  lazy: () => import(route.location)
})) : routes.map(route => ({
  path: route.path
  Component: import(route.location).Component
}))

nahtnamOP•2y ago

I don't think so, ts would've complained if i reassigned a const

lab•2y ago

import() returns a promise that eventually resolve to the module being imported at runtime

nahtnamOP•2y ago

Yeah thats what the types represent too, i'm just curious if we could somehow support top level awaits or something and have them resolve in development

lab•2y ago

this sucks bc without hoist, you ended up bundling that devtool xD I tried this and the problem is that when doing import(route.location).Component <- this breaks HMR still lol

nahtnamOP•2y ago

💀

lab•2y ago

yeah so the only option that'd allow HMR in dev and lazy load in prod is to use dedicated prod vs dev bundle file, that'd basically keep the bundler from looking at both dynamic and static import

nahtnamOP•2y ago

Hmm, let me see if there is a way I can organize my code to do that Quick question, if you add import * as Sentry from '@sentry/react'; to your newtab.ts, does building with hoist work? That line is one example of it breaking for me

lab•2y ago

if you remove it, it works?

nahtnamOP•2y ago

I commented out all other code in my newtab file Yes Well i have other stuff too

lab•2y ago

hmmm... I'm not sure what's inside the sentry package that'd break the hoisting lol

nahtnamOP•2y ago

Its not just sentry, I see several references in my error messages to supabase too

Arcane•2y ago

@nahtnam has reached level 11. GG!

nahtnamOP•2y ago

But basically if i empty my newtab file to just some plain html + import sentry it breaks that should be hopefully reproducible

lab•2y ago

barrel import might affect deduplication/tree-shaking but shouldn't affect the overall build if you do just import Sentry from '@sentry/react'; does it work? Or maybe try to import just what you need from it like init and ErrorBoundary :-?....

nahtnamOP•2y ago

node\_modules/@sentry/react/esm/index.js does not export 'default' changing to import { withProfiler } from '@sentry/react'; also doesnt work Same error message

lab•2y ago

are you using the latest version?

nahtnamOP•2y ago

Yep

lab•2y ago

they have an ESM bundle :-?

nahtnamOP•2y ago

Specifically you need to import * as and then also have export default Sentry.withProfiler(IndexNewTab, { name: 'NewTab' });

lab•2y ago

BTW is there any prereq to get the lazy of ReactRouter working? ;d I couldn't get it to render lol Getting this:

Matched leaf route at location "/lazy" does not have an element or Component. This means it will render an <Outlet /> with a null value by default resulting in an "empty" page.

Matched leaf route at location "/lazy" does not have an element or Component. This means it will render an <Outlet /> with a null value by default resulting in an "empty" page.

nahtnamOP•2y ago

Hmmm This is my export page function:

export function exportPage(props: ExportPageProps): RouteObject {
  const { layout, path, ...page } = props;

  return {
    path,
    element: layout ?? <Outlet />,
    children: [
      {
        index: true,
        ...page,
      },
    ],
  };
}

export function exportPage(props: ExportPageProps): RouteObject {
  const { layout, path, ...page } = props;

  return {
    path,
    element: layout ?? <Outlet />,
    children: [
      {
        index: true,
        ...page,
      },
    ],
  };
}

And of course, the lazy route basically needs to export a function called Component Hmm do you think this would help? https://www.npmjs.com/package/@rollup/plugin-dynamic-import-vars Oh nvm Plasmo uses parcel

lab•2y ago

yeah xddd I just added the feature to do newtab.production.tsx It actually would solve all of your problems i.e, not bundling devtool, have sentry unhoisted, and lazyload But yeah would need a slight refactor for the router ;d

nahtnamOP•2y ago

I don't mind the refactor, I'm just worried that I'd have to define each route twice because of not being able to import via a dynamic path BTW does that work with newtab/index.production.tsx? 😅

lab•2y ago

yeah it does hahah

nahtnamOP•2y ago

How would the production file solve the sentry problem? I also want it included in the production bundle

lab•2y ago

here's an example: https://github.com/PlasmoHQ/examples/blob/main/with-react-router/routes/index.production.tsx oh sentry can only be bundled without hoist to clarify - previously we needed hoist to remove the devtools, but now we can remove the devtools without hoist, which also enable bundling sentry xD

nahtnamOP•2y ago

Gotcha, so here is my game plan:

index.tsx
index.router.tsx
index.production.tsx
index.production.tsx
router.tsx

index.tsx
index.router.tsx
index.production.tsx
index.production.tsx
router.tsx

index.tsx and index.production.tsx contain the same layout, index.tsx just has the extra devtools the router.tsx has all of the routes defined BUT does not have the component definition the two .router.tsx files imports router.tsx and attaches an import statement (either lazy or not depending on which file) and I'll rely on typescript to make sure I dont miss a route

lab•2y ago

that works, you can likely infer type and import it in the other router I wonder if there's a way to do shape infer like that for route :-?. BTW @nahtnam

nahtnamOP•2y ago

Uh oh Im half way through the refactor 😂

lab•2y ago

I suspect it's possible that we will eventually have a downstream fix to the react-router and livereload xD oh lol

nahtnamOP•2y ago

Gotcha, I've made it in a way that it should be super easy to swap back

lab•2y ago

Tho in the future if it all works yeea

nahtnamOP•2y ago

I just delete the index.production.tsx and copy over the lazy router If you ever want me to test it, I can 🙂

lab•2y ago

yeet parcel will have a minor upgrade soon next month we will get their newer js transformer, and I suspect should fix this issue

nahtnamOP•2y ago

Alright here is what I ended up with

// routes.ts
export const routes = {
  '/': {},
} satisfies Record<string, RootRouteObject>;

// index.production.router
const pages = {
  '/': () => import('./app/page'),

} satisfies Record<keyof typeof routes, LazyPageRouteObject>;

export const router = createMemoryRouter([
  {
    element: <RootLayout />,
    errorElement: <ErrorFallback />,
    children: processLazyRoutes(pages),
  },
]);

// index.router
import { DevSettings } from './__components__/DevSettings';
import * as indexPage from './app/page';

const routeToPageMap = {
  '/': indexPage,
} satisfies Record<keyof typeof routes, PageRouteObject>;

export const router = createMemoryRouter([
  {
    element: (
      <RootLayout>
        <DevSettings />
      </RootLayout>
    ),
    errorElement: <ErrorFallback />,
    children: processRoutes(routeToPageMap),
  },
]);

// routes.ts
export const routes = {
  '/': {},
} satisfies Record<string, RootRouteObject>;

// index.production.router
const pages = {
  '/': () => import('./app/page'),

} satisfies Record<keyof typeof routes, LazyPageRouteObject>;

export const router = createMemoryRouter([
  {
    element: <RootLayout />,
    errorElement: <ErrorFallback />,
    children: processLazyRoutes(pages),
  },
]);

// index.router
import { DevSettings } from './__components__/DevSettings';
import * as indexPage from './app/page';

const routeToPageMap = {
  '/': indexPage,
} satisfies Record<keyof typeof routes, PageRouteObject>;

export const router = createMemoryRouter([
  {
    element: (
      <RootLayout>
        <DevSettings />
      </RootLayout>
    ),
    errorElement: <ErrorFallback />,
    children: processRoutes(routeToPageMap),
  },
]);

nahtnamOP•2y ago

Thanks for being awesome as always

lab•2y ago

noice

Gaming

Programming

react-router HMR Issues when lazy-load