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

[bug] Typescript Project References are created for unnecessary implicit dependencies #1561

Closed
rhuanbarreto opened this issue Jul 16, 2024 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@rhuanbarreto
Copy link

Describe the bug

I have the following structure:

/packages
  |-> /tool-cd
  |-> /library-b

In tool-cd (a commander CLI) that's the moon.yml:

project:
  name: CD Tools
  description: Tools for continuous delivery
platform: bun
language: typescript
type: tool
stack: infrastructure
tasks:
  cli:
    platform: bun
    command: bun index.ts
    type: run
    local: true
    options:
      persistent: false
      runInCI: false
      cache: false

In library-b that's the moon.yml:

language: typescript
platform: bun
type: library
tasks:
  test:
    deps:
      - target: tool-cd:cli
        args: azure login
      - target: tool-cd:cli
        args: keyvault allow
    options:
      runDepsInParallel: false
      mergeDeps: replace

In /.moon/tasks/typescript.yml:

fileGroups:
  sources:
    - package.json
    - src/**/*
    - "**/*.json"
    - "**/*.ts"
    - "**/*.tsx"
    - "**/*.css"
tasks:
  ... clipped ...
  test:
    platform: bun
    command: bun test
    inputs:
      - "@group(sources)"
    type: test
    deps:
      - "^:test"
    options:
      runInCI: true
      persistent: false

My toolchain file:

bun:
  syncProjectWorkspaceDependencies: true
  version: latest
  installArgs:
    - "--frozen-lockfile"
node: # Node.js is for running storybooks. They still don't support bun. https://github.com/storybookjs/storybook/issues/23279
  syncProjectWorkspaceDependencies: true
  version: v21.7.3
  packageManager: bun
typescript:
  createMissingConfig: true
  syncProjectReferences: true
  syncProjectReferencesToPaths: true
  routeOutDirToCache: true

Because of those dependencies, the library-b gets an implicit dependency of scope build on tool-cd and the syncProjectReferences config add references in tsconfig.json. But this is actually not needed since I don't need the CLI to build the library with tsc, which increases time spent in running moon ci. This also generates issues with circular dependencies if tool-cd depends on the library to run and the library needs to run a command in order to initialize unit tests.

The only way to solve this is by splitting the tasks in moon.yml for library-b:

language: typescript
platform: bun
type: library
tasks:
  azure-login:
    local: true
    type: run
    platform: bun
    command: bun ../tool-cd/index.ts azure login
    options:
      persistent: false
  keyvault-allow:
    local: true
    type: run
    platform: bun
    command: bun ../tool-cd/index.ts keyvault allow
    options:
      persistent: false
  test:
    deps:
      - azure-login
      - keyvault-allow
    options:
      runDepsInParallel: false
      mergeDeps: replace
@milesj
Copy link
Collaborator

milesj commented Jul 17, 2024

This still an issue in 1.27.1?

@rhuanbarreto
Copy link
Author

The issue is fixed! Thanks a lot!

But there's a side effect that will probably become a feature request.

Now when I remove the dependency in tsconfig.json it doesn't get recreated, which is good. But it's very easy to lose track of all the places you need to remove. And the moon sync projects command is addition-only. It doesn't remove what's not needed anymore. I know people want to have more control over this, but I'm in "autopilot" mode here, delegating all this to moon.

So maybe there could be a flag like moon sync projects --full to remove all inside "compilerOptions.paths" and "references" and fill up from scratch? I had to do this manually after the fix.

@milesj
Copy link
Collaborator

milesj commented Jul 17, 2024

That makes sense, but the issue is that moon doesn't know what the previous state was. If there was 5 projects before, but only 4 now, moon doesn't know which 1 was removed. For paths this is super complicated, as the user may have added their own paths. For references though, we could just replace the entire list each time.

@rhuanbarreto
Copy link
Author

That's why the opt-in flag. If you use this flag you assume the content will be fully overwritten. The normal behaviour would keep the same. This would save lots of time to keep up with changes in a growing monorepo.

@rhuanbarreto
Copy link
Author

Fixed.

Follow up in #1579

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

2 participants