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

[6.10.0][Bug]: tree shaking is not implemented #10354

Closed
xr0master opened this issue Apr 16, 2023 · 9 comments
Closed

[6.10.0][Bug]: tree shaking is not implemented #10354

xr0master opened this issue Apr 16, 2023 · 9 comments
Labels

Comments

@xr0master
Copy link

What version of React Router are you using?

6.10.0

Steps to Reproduce

It looks like the tree shaking is not implemented. The library is huge.

version 5
Screenshot 2023-04-16 at 23 39 20

version 6
Screenshot 2023-04-16 at 23 38 01

Expected Behavior

"In v6, the size of the minified gzipped bundle is reduced by more than 50%. so, React Router currently contributes less than 4kb of your overall app bundle."

Actual Behavior

v6 is more than 5 times larger than v5

@xr0master xr0master added the bug label Apr 16, 2023
@timdorr
Copy link
Member

timdorr commented Apr 17, 2023

Tree-shaking is a bundler matter, not a concern of the library. We export modules, which your bundler can import and tree-shake appropriately. The entirety of the library will rarely be fully included in any bundle. If it is, you have a bundler configuration issue.

@timdorr timdorr closed this as completed Apr 17, 2023
@gerhardsletten
Copy link

So to clarify, the bundle size has doubled in v6:

bundlephobia.com/package/[email protected] - 63.8 kB (20 kB gzip)

vs v5:

bundlephobia.com/package/[email protected] - 30.7 kB 9.8 kB gzip)

Are there any plans to do something with this?

@xr0master
Copy link
Author

@timdorr I did not expect another answer/action from Remix Run (away). Most use CRA for React, and I doubt that there is a problem with the bundler.

Of course, instead of analyzers, we can use some "hello world" project and look at the real numbers.

@gerhardsletten
Copy link

@xr0master Looking into this, and if you avoid data-routers, the bundle-size will stay around what is used to be in react-router#v5. In the app I work with, we already use react-query for data-loading, and we did use react-router-config (in v5) to figure out what data to load on the server. Moving to v6, the same can be accomplished with matchRoutes and createRoutesFromElements

Migrating to react-router v6 you can store your applications routes in a var, and use the Route.handle as a key on what data-fetching done in the server:

import React from 'react'
import { Route } from 'react-router-dom'

export const routes = (
  <Route path="/" Component={App} handle="app">
    <Route index Component={HomePage} handle="homepage" />
    <Route path="nested" Component={SubPage} handle="subpage" />
  </Route>
)

On the client this can be rendered like this:

import { BrowserRouter, Routes } from 'react-router-dom'
return (
    <BrowserRouter>
      <Routes>{routes}</Routes>
    </BrowserRouter>
  )

And on the server:

import { StaticRouter } from 'react-router-dom/server'
import { matchRoutes, createRoutesFromElements, Routes } from 'react-router-dom'
import App from 'containers/App'
import HomePage from 'containers/HomePage'
import SubPage from 'containers/SubPage'

const renderApp = async (req) => {
  const location = new URL(req.url, config.appUrl)
  const { pathname } = location
  const queryClient = new QueryClient()
  const layered = filterComponents(
    matchRoutes(createRoutesFromElements(routes), pathname)
  )
  await loadData(layered, { queryClient, location })
  const component = (
    <StaticRouter location={req.url}>
      <Routes>{routes}</Routes>
    </StaticRouter>
  )
  // ...
}

const pageList = {
  app: App,
  homepage: HomePage,
  subpage: SubPage
}

function filterComponents(branch) {
  return branch.reduce((result, { route, match }) => {
    if (route.Component && route.handle && pageList[route.handle]) {
      const comp = pageList[route.handle]
      if (comp && comp.ssrPrefetch) {
        result.push([comp, { route, match }])
      }
    }
    return result
  }, [])
}
async function loadData(components, { queryClient, location }) {
  const promises = components.map(([comp, ownProps]) =>
    comp.ssrPrefetch({ queryCache: queryClient, location, ...ownProps })
  )
  await Promise.all(promises)
}

We have a ssrPrefetch function on each Routing-components which will load data for the current page:

async function ssrPrefetch({ queryCache,location) {
  return Promise.all([
    queryCache.prefetchQuery(['page', location], () => loadPage(location)),
  ])
}
App.ssrPrefetch = ssrPrefetch

@xr0master
Copy link
Author

@gerhardsletten thanks for the tip! In fact, if avoid using the data router, then the size is significantly reduced. 398.14 kB (-10.17 kB).

@gerhardsletten
Copy link

@xr0master I also made an Vite example with an updated way to handle both lazy Routes modules and data fetching with react-query here: github.com/gerhardsletten/vite-react-router-ssr-suspend

@khoroshevj
Copy link

still an issue, huge size impact of @remix-run/router in bundle, using react-router-dom version 6.23.1 with Vite. any workaround? using just these createBrowserRouter, useParams, Outlet, Link, Navigate, RouteObject, RouterProvider from react-router-dom

Screenshot 2024-05-23 at 14 11 00 Screenshot 2024-05-23 at 14 17 44 Screenshot 2024-05-23 at 14 20 49

@sphinxrave
Copy link

@khoroshevj , I reproduced this in bundlejs here:

https://bundlejs.com/?q=react-router-dom%2Creact-router-dom&treeshake=%5B*%5D%2C%5B*%5D&text=%22createBrowserRouter%28%29%3B%5CnNavigate%3B%5Cn%5CnLink%3B%22&config=%7B%22analysis%22%3A%22treemap%22%2C%22esbuild%22%3A%7B%22external%22%3A%5B%22react%22%2C%22react-dom%22%5D%7D%7D

Do note though that the analysis might be off, since it reports 15kb gzip full but the treemap suggests 72kb gzip full, I'm not sure which to believe?

image
image

@TimVerheul
Copy link

Also having this issue. Not sure why it is closed. 173kb router.js file which I can't seem to get splitted

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants