vercel / next.js

The React Framework
https://nextjs.org
MIT License
127.02k stars 26.99k forks source link

Code elimination broken for module that exports frontend and Node.js #16153

Closed timsuchanek closed 1 year ago

timsuchanek commented 4 years ago

Bug report

Describe the bug

The code elimination is broken, if you import from a file, which exports both frontend and Node.js code.

To Reproduce

Clone and run: https://github.com/timsuchanek/build-cv-messing-around

Expected behavior

The code elimination should sort out the frontend and Node.js code correctly.

Screenshots

image

System information

Additional context

This is an isolated reproduction of a problem a Prisma user hit with running Prisma Client in getStaticProps https://github.com/prisma/prisma/issues/3252

timneutkens commented 4 years ago

As of right now it's not possible to have both client-side JS and Node.js-specific code as the tree shaking of getStaticProps / getStaticPaths works on the module level, it's not a whole program optimization. You can see what is removed and what is not here: https://next-code-elimination.now.sh/

The elimination is only applied to page files in the pages directory.

timsuchanek commented 4 years ago

Btw in this case, we're not even having frontend code, but just an import of a json object, that is unused. Maybe fixing this case could already be a good first step.

timsuchanek commented 4 years ago

Is it possible to test the scenario I described here in https://next-code-elimination.now.sh/ ? From what I see, it would need a separate pane in there to be able to build a separate module.

adarnon commented 4 years ago

I've also been experiencing that using Prisma. I'm exposing a basic REST API from my Next.js backend, and I want to just use my Prisma model types for static type checking. My workaround now is to redefine my Prisma models as TypeScript types in another file. Would be nice to not have to maintain this code duplication.

fimbault commented 4 years ago

I've tested that under a slightly different setup, using SQLite as a sort of extreme case (to avoid backend calls for data that is potentially configurable, like in a CMS - updates are done through git push -, but runtime access in readonly mode and without having to maintain it on a central server). In some settings it works, in others it doesn't.

Here's an example of stack trace when it doesn't work:

error - ./node_modules/@prisma/client/runtime/index.js:87:2855 Module not found: Can't resolve 'async_hooks' null Could not find files for / in .next/build-manifest.json

so it fails on the instruction {AsyncResource:Iy}=required("async_hook")

Note : I retried this morning, and the example works. So I'm not sure what happened. If I can reproduce, I'll publish a code example.

denis-sokolov commented 2 years ago

A simple scenario that triggers this bug is moving the contents of a pages/index.tsx file into a separate file. A refactoring that is expected to be inherently equivalent breaks the build:

// Before
// pages/index.tsx:
import fs from 'fs';
export default function Home(){ return <div />; }
export async function getServerSideProps(){ /* some work using fs */ }

// After
// pages/index.tsx:
export { default as default, getServerSideProps } from '../helper.tsx';
// helper.tsx:
import fs from 'fs';
export default function Home(){ return <div />; }
export async function getServerSideProps(){ /* some work using fs */ }
magnattic commented 2 years ago

@denis-sokolov Did you find a workaround?

denis-sokolov commented 2 years ago

I stopped exporting frontend and Node.js code from the same file. ¯\_(ツ)_/¯

magnattic commented 2 years ago

Not sure I understand. In the example you gave above, were are you exporting frontend and Node.js code from the same file?

I am trying to do the same, move code from the pages folder somewhere else, then import and re-export it in pages. Getting

Module not found: Can't resolve 'fs'

denis-sokolov commented 2 years ago

I put front-end code and node.js code into separate files:

export { HomePage as default } from 'views/Home';
export { getServerSideProps } from 'views/Home/server';
magnattic commented 2 years ago

So there is no way currently to leave the getServerSideProps in the same file as the page code if I want to put them into a directory that is not pages? Disappointing :(

lesderid commented 1 year ago

Is this still not fixed (i.e. eliminating code in files outside the pages directory)? We have to add a bunch of extra files (e.g. foobar-client.js and foobar-server.js) to get around this.

timneutkens commented 1 year ago

For App Router we've kept this issue in mind and changed the approach to no longer magically shake like with getServerSideProps / getStaticProps. Instead, the React Server Components model requires a compiler-level marker called "use client" to mark the boundary between server components and client components. This ensures that your code has to be split between server-only (server components) and server+client (client components). In case of prisma you'd add import 'server-only' to the initializer file as well to provide a compiler hint to throw a compilation error when trying to import files that are server-only in client components.

github-actions[bot] commented 1 year ago

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.