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

Research Solution for Circular Dependency Checking #11

Closed
jakobo opened this issue Sep 29, 2022 · 1 comment
Closed

Research Solution for Circular Dependency Checking #11

jakobo opened this issue Sep 29, 2022 · 1 comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed research Investigative tasks that require someone to dig in and share findings w/ the community

Comments

@jakobo
Copy link
Owner

jakobo commented Sep 29, 2022

Problem Statement

madge isn't great with Typescript circular dependencies. We use it right now to avoid the most egregious problems, but there's likely better solutions since the original build scripts were written.

Proposed Solution

This is a research task, which means no code is required (unless you want to do a PR). There's a set of questions we're hoping to answer, informing the project direction.

  1. What value are we getting from madge? How could we introduce a circular dependency in DocMQ, and can we trust madge to catch it?
  2. What alternatives to madge are out there? Are they better than what we currently have? I found two quickly googling, but this isn't an exhaustive list by any means: dpdm, eslint import rule
  3. Is it worth keeping madge, swapping to an alternative, or removing the circular dependency checks altogether?
@jakobo jakobo changed the title Swap mage for alternative Swap madge for alternative Sep 29, 2022
@jakobo jakobo changed the title Swap madge for alternative Research Solution for Circular Dependency Checking Sep 29, 2022
@jakobo jakobo added help wanted Extra attention is needed good first issue Good for newcomers research Investigative tasks that require someone to dig in and share findings w/ the community labels Sep 29, 2022
@jakobo
Copy link
Owner Author

jakobo commented Sep 29, 2022

To help out whoever takes this on. For (1), the main value we get is to avoid circular dependencies when using import statements. It's really easy to forget a type keyword on an import statement and end up with code that doesn't build.

// a.js
import { BType } from "b.js" // <= should've been a type import

// 2
import { AType } from "a.js"" // <= should've been a type import

In this example, the ESM code will import BType and AType as objects instead of types. While Typescript can manage the circular dependency and sees these as types, node.js running in ESM mode will not and one of these modules will resolve to {} because it must be evaluated first.

This isn't an issue in CommonJS, which does all checks in a single pass (and lets us assign to module.exports after the fact).

@jakobo jakobo mentioned this issue Nov 30, 2022
5 tasks
@jakobo jakobo closed this as completed in 67322e9 Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed research Investigative tasks that require someone to dig in and share findings w/ the community
Projects
None yet
Development

No branches or pull requests

1 participant