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

expiring-todo-comments package conditions don't work in a multi-package project layout #403

Closed
ark120202 opened this issue Sep 29, 2019 · 5 comments · Fixed by #2159
Closed

Comments

@ark120202
Copy link

Currently expiring-todo-comments loads package.json data during initial load and resolves it from working directory. However, in case of monorepo-structured project, linted files may have a closer package.json file. Not sure how much that would affect performance, but I think it makes sense to resolve it for every file individually.

@MrHen
Copy link
Contributor

MrHen commented Sep 30, 2019

Can you post a specific example of your monorepo structure? In the monorepo structure I've used, the linting runs are always done at the project level -- not at the monorepo level.

I'm personally okay letting this be an exception but we could call it out in the documentation.

@ark120202
Copy link
Author

Most of open source monorepos I've seen appear to use it that way:

@MrHen
Copy link
Contributor

MrHen commented Sep 30, 2019

Awesome, thanks for the links. I guess the monorepos I've maintained were cross-language so we couldn't use one tool for all of included the projects.

@happycollision
Copy link

happycollision commented Sep 29, 2021

I just ran into this problem. I actually have two use-cases within a single repo. We have a monorepo where much of the root package.json defines only the lowest-level dependencies, while other packages define very common dependencies, and leaf packages define specific dependencies.

Something like this

root/
  │
  ├─packages/
  │   │
  │   ├─monitor/
  │   │   │┌─────────────────┐
  │   │   └│package.json     │
  │   │    │  @sentry/browser│
  │   │    └─────────────────┘
  │   └─app/
  │       │┌─────────────────┐
  │       └│package.json     │
  │        │  react          │
  │        │  tailwindcss    │
  │        └─────────────────┘
  │┌────────────────┐
  └│package.json    │
   │  @babel/core   │
   └────────────────┘

The app package depends on monitor, and both use @babel packages.

Inside app, I would have wanted to do both of the following:

  1. TODO [+@sentry/browser]: after adding sentry, we can log these errors...
  2. TODO [+tailwindcss]: after adding tailwind, refactor away these css file imports

The first has knowledge of a dependency in another package, which is fine in a monorepo in my opinion—especially in the comments. The second has knowledge of its own dependencies.

Neither of these work because the rule only sees the root package's dependencies (@babel, etc).

@andymac4182
Copy link

Any update on this?
#2159 seems to resolve this. This is preventing us from adopting this check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants