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

WIP: allow circular project references #54216

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Zzzen
Copy link
Contributor

@Zzzen Zzzen commented May 11, 2023

Fixes #33685

This issue is still being discussed, so there's no need to delve into it here. However, I would like to proceed with packing it and migrating our current project to project references, in order to prepare for the eventual elimination of circular references.

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label May 11, 2023
@typescript-bot
Copy link
Collaborator

The TypeScript team hasn't accepted the linked issue #33685. If you can get it accepted, this PR will have a better chance of being reviewed.

@Zzzen
Copy link
Contributor Author

Zzzen commented May 12, 2023

@typescript-bot pack this

@Zzzen
Copy link
Contributor Author

Zzzen commented May 12, 2023

@jakebailey Could you help me pack it?

@jakebailey
Copy link
Member

I mean, I guess I can, but your PR comment implies that you're wanting us to pack this so you can use it in production, which feels a little concerning. Packed builds are really only for testing and to give to people who are having problems to make sure the fix works. If you are actually wanting to use this for anything else... it feels like you probably are actually just trying to work on a continued fork of TS? We're can't keep packing this over and over to support that use case...

I also don't think it's "eventual" that we do this, either.

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 12, 2023

Heya @jakebailey, I've started to run the tarball bundle task on this PR at f4923a4. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 12, 2023

Hey @jakebailey, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/154201/artifacts?artifactName=tgz&fileId=23414506DC401DC1E74E22A39B5F732972DEAE572B3BDB56AF931DE9D643415102&fileName=/typescript-5.1.0-insiders.20230512.tgz"
    }
}

and then running npm install.


There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@5.1.0-pr-54216-5".;

@Zzzen
Copy link
Contributor Author

Zzzen commented May 13, 2023

Thank you for raising your concerns. I understand your point of view, but let me clarify our intentions. Yes, we do want to use this in production, but we're also actively working on refactoring our code to eliminate circular dependencies. This version should help us through the refactoring process, and we don't want to maintain a fork of the TypeScript compiler either, will switch back to the official versions once it's done.

@Zzzen Zzzen marked this pull request as draft May 13, 2023 03:11
@ozyman42
Copy link

Could we add a compiler option which keeps the existing logic in place (fails if any circular dependency exists)?

@Zzzen
Copy link
Contributor Author

Zzzen commented Jun 24, 2023

If I'm understanding correctly, it seems that no additional options are required because it will still fail unless all circular dependencies are explicitly identified as such.

1 similar comment
@Zzzen
Copy link
Contributor Author

Zzzen commented Jun 24, 2023

If I'm understanding correctly, it seems that no additional options are required because it will still fail unless all circular dependencies are explicitly identified as such.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow circular references of project references
4 participants