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

Internal: package dependency cycles #9712

Closed
SimenB opened this issue Mar 26, 2020 · 12 comments
Closed

Internal: package dependency cycles #9712

SimenB opened this issue Mar 26, 2020 · 12 comments

Comments

@SimenB
Copy link
Member

SimenB commented Mar 26, 2020

Noticed these when publishing a release

lerna WARN ECYCLE Dependency cycles detected, you should fix these!
lerna WARN ECYCLE jest-circus -> jest-runtime -> jest-config -> @jest/test-sequencer -> jest-runner -> jest-circus
lerna WARN ECYCLE jest-jasmine2 -> (nested cycle: jest-circus -> jest-runtime -> jest-config -> @jest/test-sequencer -> jest-runner -> jest-circus) -> jest-jasmine2
lerna WARN ECYCLE (nested cycle: jest-jasmine2 -> (nested cycle: jest-circus -> jest-runtime -> jest-config -> @jest/test-sequencer -> jest-runner -> jest-circus) -> jest-jasmine2) -> (nested cycle: jest-jasmine2 -> (nested cycle: jest-circus -> jest-runtime -> jest-config -> @jest/test-sequencer -> jest-runner -> jest-circus) -> jest-jasmine2)
lerna WARN ECYCLE Dependency cycles detected, you should fix these!
lerna WARN ECYCLE jest-config -> @jest/test-sequencer -> jest-runner -> jest-config
lerna WARN ECYCLE jest-jasmine2 -> jest-runtime -> (nested cycle: jest-config -> @jest/test-sequencer -> jest-runner -> jest-config) -> jest-jasmine2
lerna WARN ECYCLE (nested cycle: jest-jasmine2 -> jest-runtime -> (nested cycle: jest-config -> @jest/test-sequencer -> jest-runner -> jest-config) -> jest-jasmine2) -> (nested cycle: jest-jasmine2 -> jest-runtime -> (nested cycle: jest-config -> @jest/test-sequencer -> jest-runner -> jest-config) -> jest-jasmine2)
lerna WARN ECYCLE Dependency cycles detected, you should fix these!
lerna WARN ECYCLE jest-config -> @jest/test-sequencer -> jest-runner -> jest-config
lerna WARN ECYCLE jest-jasmine2 -> jest-runtime -> (nested cycle: jest-config -> @jest/test-sequencer -> jest-runner -> jest-config) -> jest-jasmine2
lerna WARN ECYCLE (nested cycle: jest-jasmine2 -> jest-runtime -> (nested cycle: jest-config -> @jest/test-sequencer -> jest-runner -> jest-config) -> jest-jasmine2) -> (nested cycle: jest-jasmine2 -> jest-runtime -> (nested cycle: jest-config -> @jest/test-sequencer -> jest-runner -> jest-config) -> jest-jasmine2)

I think it's types causing these, but haven't investigated

@WeiAnAn
Copy link
Contributor

WeiAnAn commented Apr 7, 2020

I found these two place of type import causing dependency cycles.

https://github.com/facebook/jest/blob/2edd5c4724cfadb944a6a233a7ad8fb5a3e05996/packages/jest-test-sequencer/src/index.ts#L11-L12
https://github.com/facebook/jest/blob/2edd5c4724cfadb944a6a233a7ad8fb5a3e05996/packages/jest-jasmine2/src/index.ts#L13

I try to move dependencies to devDependencies in package.json, but lerna still show the warning.
It work with removing those dependencies.
I'm not sure if it cause any error. I think we have references config in tsconfig.json, it should work fine.

@ghostd
Copy link
Contributor

ghostd commented May 9, 2020

It seems we have something like this:
jest-jasmine2 needs jest-runtime, jest-runtime needs jest-config, jest-config needs jest-jasmine2 to load the test runner in normalize.ts
https://github.com/facebook/jest/blob/8294bab1ebbf9bac7d2aa01abd256bbca4952eb7/packages/jest-config/src/normalize.ts#L557-L559

@SimenB
Copy link
Member Author

SimenB commented May 9, 2020

Thanks for digging into it!

jest-runtime shouldn't need jest-config (nothing except @jest/core and possibly jest-cli should need jest-config). It seems it's just for the runtime CLI. I think we should just split out jest-runtime-cli to a separate module in Jest 27. I highly doubt anyone is using it, so might even just delete it...

jest-jasmine2 (and jest-circus) needs jest-runtime for its types.

@SimenB
Copy link
Member Author

SimenB commented May 9, 2020

I opened up #10011 for that one.

jest-runner depends on jest-config for getTestEnvironment - should probably just make this less magical and just depend require these modules to be fully named (discussed in #5913)

@SimenB
Copy link
Member Author

SimenB commented Jul 30, 2020

As of 26.2.0, FWIW

lerna WARN ECYCLE Dependency cycles detected, you should fix these!
lerna WARN ECYCLE jest-circus -> jest-runner -> jest-circus
lerna WARN ECYCLE jest-config -> @jest/test-sequencer -> (nested cycle: jest-circus -> jest-runner -> jest-circus) -> jest-config
lerna WARN ECYCLE jest-jasmine2 -> jest-runtime -> (nested cycle: jest-config -> @jest/test-sequencer -> (nested cycle: jest-circus -> jest-runner -> jest-circus) -> jest-config) -> jest-jasmine2
lerna WARN ECYCLE (nested cycle: jest-jasmine2 -> jest-runtime -> (nested cycle: jest-config -> @jest/test-sequencer -> (nested cycle: jest-circus -> jest-runner -> jest-circus) -> jest-config) -> jest-jasmine2) -> (nested cycle: jest-jas
mine2 -> jest-runtime -> (nested cycle: jest-config -> @jest/test-sequencer -> (nested cycle: jest-circus -> jest-runner -> jest-circus) -> jest-config) -> jest-jasmine2)
lerna WARN ECYCLE Dependency cycles detected, you should fix these!
lerna WARN ECYCLE jest-config -> @jest/test-sequencer -> jest-runner -> jest-config
lerna WARN ECYCLE jest-jasmine2 -> jest-runtime -> (nested cycle: jest-config -> @jest/test-sequencer -> jest-runner -> jest-config) -> jest-jasmine2
lerna WARN ECYCLE (nested cycle: jest-jasmine2 -> jest-runtime -> (nested cycle: jest-config -> @jest/test-sequencer -> jest-runner -> jest-config) -> jest-jasmine2) -> (nested cycle: jest-jasmine2 -> jest-runtime -> (nested cycle: jest-c
onfig -> @jest/test-sequencer -> jest-runner -> jest-config) -> jest-jasmine2)
lerna WARN ECYCLE Dependency cycles detected, you should fix these!
lerna WARN ECYCLE jest-config -> @jest/test-sequencer -> jest-runner -> jest-config
lerna WARN ECYCLE jest-jasmine2 -> jest-runtime -> (nested cycle: jest-config -> @jest/test-sequencer -> jest-runner -> jest-config) -> jest-jasmine2
lerna WARN ECYCLE (nested cycle: jest-jasmine2 -> jest-runtime -> (nested cycle: jest-config -> @jest/test-sequencer -> jest-runner -> jest-config) -> jest-jasmine2) -> (nested cycle: jest-jasmine2 -> jest-runtime -> (nested cycle: jest-c
onfig -> @jest/test-sequencer -> jest-runner -> jest-config) -> jest-jasmine2)

@ghostd
Copy link
Contributor

ghostd commented Jan 2, 2021

After some "grepping", i found this dependencies cycle:

  • jest-runner needs jest-config for getTestEnvironment (called by runTestInternal)
  • jest-config needs jest-circus to load the test runner with options.testRunner = require.resolve('jest-circus/runner');
  • jest-circus needs jest-runner to import the type TestFileEvent (in jestAdapterInit.ts, jestAdapter.ts, and testCaseReportHandler.ts);
  • we have a loop ;-)

@SimenB any advices to break this cycle?

@SimenB
Copy link
Member Author

SimenB commented Jan 6, 2021

We could move getTestEnvironment to jest-util I guess?

@ghostd
Copy link
Contributor

ghostd commented Jan 6, 2021

Ok, i'll take a look this weekend.

@ghostd
Copy link
Contributor

ghostd commented Jan 6, 2021

I quickly looked over. If we move getTestEnvironment in jest-utils, we also have to move resolveWithPrefix with it, but this function needs Resolver from the jest-resolve package... which depends on jest-utils... we have created an other cycle :)

Maybe we can move getTestEnvironment and resolveWithPrefix into jest-resolve, i can give a shot. Does it make sense to you, or this is not pertinent?

@SimenB
Copy link
Member Author

SimenB commented May 2, 2021

Moving resolveWithPrefix into jest-resolve seems fine. getTestEnvironment seems to not belong there, though... Let's start with resolveWithPrefix?

@SimenB
Copy link
Member Author

SimenB commented May 29, 2021

Resolved in #11466

@SimenB SimenB closed this as completed May 29, 2021
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants