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

circular dependency in next\dist\lib\router\index.js #5392

Closed
Enalmada opened this issue Oct 7, 2018 · 6 comments
Closed

circular dependency in next\dist\lib\router\index.js #5392

Enalmada opened this issue Oct 7, 2018 · 6 comments

Comments

@Enalmada
Copy link
Contributor

Enalmada commented Oct 7, 2018

I am trying to build a zip file only containing the code I need for production deployment. Smaller builds benefit nearly all production users: cheaper to store indefinitely, quicker to roll out after build finish, faster to load cold start in serverless, and for large projects it is easy to go over serverless dist limits without optimized distribution. I believe rollout is the best option for getting an optimized build.

The good news, running rollup turns my 60m dist file into 6m. I believe this concept will be a huge win for people who want their next.js app to start as quick as possible.

Bad news: When I try to run rollup on my next.js build, I get a circular dependency issue that I believe stops the result from running:

(!) Circular dependency: node_modules\next\dist\lib\router\index.js -> node_modules\next\dist\lib\router\router.js -> node_modules\next\dist\lib\router\index.js
(!) Circular dependency: node_modules\next\dist\lib\router\index.js -> node_modules\next\dist\lib\router\router.js ->  commonjs-proxy:<FULL_PATH_TO_HERE>\node_modules\next\dist\lib\router\index.js -> node_modules\next\dist\lib\router\index.js

Would it be possible to not have a circular dependency between index.js and routes.js?

Here are the files I am using for testing this, hopefully this might help someone more experienced carry on from where I am stuck:

rollup.config.js

import babel from "rollup-plugin-babel";
import commonjs from "rollup-plugin-commonjs";
import resolve from "rollup-plugin-node-resolve";
import replace from "rollup-plugin-replace";
import json from 'rollup-plugin-json';

const NODE_ENV = process.env.NODE_ENV || "development";
const outputFile = NODE_ENV === "production" ? "./lib/prod.js" : "./lib/dev.js";

export default {
    input: "./server.js",
    output: {
        file: outputFile,
        format: "cjs"
    },
    plugins: [
        replace({
            "process.env.NODE_ENV": JSON.stringify(NODE_ENV)
        }),
        json(),
        babel({
            exclude: ["node_modules/**"]
        }),
        resolve({
            preferBuiltins: true
        }),
        commonjs()
    ],
    external: ["webpack", "readable-stream", "glob", "source-list-map", "through2" ]
};

(external has a few exclusions due to circular dependencies that need to be taken up with those package owners.)

"devDependencies": {
    ...
    "rollup": "^0.66.4",
    "rollup-plugin-babel": "^4.0.3",
    "rollup-plugin-commonjs": "^9.1.8",
    "rollup-plugin-json": "^3.1.0",
    "rollup-plugin-node-resolve": "^3.4.0",
    "rollup-plugin-replace": "^2.1.0"
  }

create rollup (note circular dependency issue)

NODE_ENV=production rollup -c

to run

node lib/prod.js

error I get but I suspect it is because of circular dependency

C:\Users\Adam\code\gell\gell-client-west\lib\prod.js:135129
const childModule  = commonjsRequire.resolve('./child/index');
                                     ^

TypeError: commonjsRequire.resolve is not a function
    at Object.<anonymous> (lib\prod.js:135129:38)
    at Module._compile (internal/modules/cjs/loader.js:689:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
    at Module.load (internal/modules/cjs/loader.js:599:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:538:12)
    at Function.Module._load (internal/modules/cjs/loader.js:530:3)
    at Function.Module.runMain (internal/modules/cjs/loader.js:742:12)
    at startup (internal/bootstrap/node.js:279:19)
    at bootstrapNodeJSCore (internal/bootstrap/node.js:752:3)

Can anyone weigh in on:

  • how hard it would be to rewrite next\dist\lib\router\index.js -> next\dist\lib\router\router.js without circular dependency? Could the router.js code just be included directly into index.js?
  • Is "commonjsRequire.resolve is not a function" error due to the circular dependency or something else?
@romainquellec
Copy link
Contributor

Is this issue will be fixed with next-server ? @timneutkens

@timneutkens
Copy link
Member

timneutkens commented Dec 31, 2018

Closing this in favor of #5927 (the serverless target will output much smaller bundles than the solution outlined in this issue would)

@Enalmada
Copy link
Contributor Author

Enalmada commented Dec 31, 2018

The current size of my built next directory is over 500m because of some huge node_modules containing unnecessary code. I currently run the next build result on Amazon Beanstalk but the sheer size of the builds causes extra storage cost, and startup delay. It is great that next.js serverless is getting smaller, but I believe this ticket should remain open for those not able to use serverless yet (websockets, node_modules too big for now v2 disk limits, etc). Is it possible to leverage similar principles from 5927 that make serverless smaller to get smaller bundles using the traditional next build so people doing node server.js solution have smaller build size?

@isaachinman
Copy link
Contributor

@timneutkens I'm setting up rollup for next-i18next and have noticed the same circularity warning. I'm on Next v7.0.2. Is this something that's coming in the 8.x release?

@timneutkens
Copy link
Member

@isaachinman you should mark the next imports as external, otherwise rollup will bundle them up and you end up with duplicate code.

@isaachinman
Copy link
Contributor

isaachinman commented Feb 4, 2019

Makes sense, thanks.

@lock lock bot locked as resolved and limited conversation to collaborators Feb 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants