Next.js: withRouter lost hoist-non-react-statics feature

[tangye1234]

Bug report

withRouter HOC cannot hoist non-react statics

Describe the bug

I found that canary.20 removes that feature for a reduction of packing. But it is a breaking change and many projects depending on it could be broken with that change.

System information

• Version of Next.js: [v8.0.4-canary.20 and later versions]

[timneutkens]

It's not really breaking from the framework side of things, eg our tests passed, so it means you're doing something super non-standard, getInitialProps is still copied. There is no good reason to ship an extra 4KB of client-side JS for an edge-case. If you want to support non-standard behavior you can use hoist-non-react-statics on the wrapped element, however I wouldn't recommend using it as, as said, it pulls in 4KB of JS to do something you generally don't want to do.

[tangye1234]

@timneutkens, thanks for a quick reply, I agree with that. But I need to modify all my pages in deed since I bound custom statics in my pages via HOC and apply the statics in _app.js, that is a huge work.

[holmesal]

@timneutkens our team ran into this as well. We're following this example from the next.js examples/ folder, but with the above change, the static query = ... gets dropped by withRouter.

What would you recommend in this case? We're planning to use a custom withRouter that doesn't drop our statics in some places, and remove withRouter in some others - but it doesn't feel like a great solution.

I get where you're coming from RE: bundle size, but I think a common expectation when using an HOC like withRouter is that it doesn't mess with your component beyond adding some props, and generally Just Works.

[exogen]

It's not really breaking from the framework side of things, eg our tests passed, so it means you're doing something super non-standard, getInitialProps is still copied

Attaching statics is not super non-standard. I've seen it in nearly every React project I've worked on.

I just wanna say that it's really disappointing that a framework that serves as a foundational piece of many people's stacks is developed in such a move-fast-and-break-things style. Trust is eroding here; I'm not the only one constantly griping about it. Is the best way to influence the development process just to try out canary releases more often? With much love and respect for everything you do: I'd really like it if I didn't have to frequently apologize on behalf of the framework I recommend to people because it breaks stuff all the time, know what I mean? 😅

[timneutkens]

I just wanna say that it's really disappointing that a framework that serves as a foundational piece of many people's stacks is developed in such a move-fast-and-break-things style.

"Move fast and break things" as said we have a very elaborate test suite. If something you're doing isn't covered you can add a test for it to prevent breakage in the future. In this case we tested against zeit.co and many other consumers and nothing broke. Hoisting statics is non-standard, it copies values. Not to mention in doing so it bloats every single bundle. Also it was never documented as copying values.

Furthermore we have canary releases for months before releasing a new version.

[holmesal]

@timneutkens I don't think it's non-standard. It's in the react HOC docs under the heading "Static Methods Must Be Copied Over".

The hoist-non-react-statics npm package is downloaded 6M times a week - more than react itself. This isn't a niche pattern - it's the react-recommended way to write an HOC that doesn't break the components you're wrapping it around. Yes, it's a workaround - but tons of people rely on it.

Bottom line - people that use withRouter are going to expect it to Just Work, like every other HOC that they use on a daily basis (react-router, react-redux, and lots of other popular packages).

Please at least add a warning to withRouter noting that it doesn't work with statics, so that people don't have to google for issues like this to figure out why adding withRouter broke things.

Again, thanks for all your awesome work on Next 👍

[tangye1234]

withRouter indeed breaks things without hoist-non-react-statics. Currently people who don't know that finally will find explanations here, because those projects does not work when updating next.js 8.1.

Awesome work on Next, and we can change our coding to fit that. But best is to notice people with that change.

[AliasT]

Any progress here ?