-
Notifications
You must be signed in to change notification settings - Fork 459
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
test(custom-compiler): deprecated ttypescript and use ttsc #4074
Conversation
Pull Request Test Coverage Report for Build 4549846699
💛 - Coveralls |
Do I need to modify the legacy side as well? |
Yes pls do because that is the codes currently running, even though it’s called legacy but it is the actual main source codes |
This will not work. See below. |
Make sure to present solid evidence and arguments in the linked issue. If you have enough evidence, I will withdraw the promotion. |
I'll be diving into the relevant conversations, which seem to be spread across repos, after work today to get an understanding of the situation. Let's please hold off on merging until then. |
@kulshekhar fwiw, we expect to release the new version of tsp next week, which will also have drop-in replacement for ttsc. Only difference for migration purposes is the command: which will be This debacle is a great illustration of that, unfortunately. We have regex replace doing code manipulation, which is bad enough. There is also no throw if it fails, leading to situations like this where people think it's working because it doesn't throw. The proper and most future-proof solution is through AST manipulation, which we've implemented. The new ts5 compilation method and output structure essentially requires this for various reasons. To mitigate perf differences, we've also introduced caching, which should ultimately improve speed over former tsp and ttypescript after the first run. Any other questions, feel free to ask. Sorry for the drama making it to your inbox! |
Dear ts-jest developers, I apologize for having this conversation here. Please feel free to ignore this comment. And if the structure changes further as the TS version goes up, that ttsc will most likely not work (it will be broken). The context is going to be a lot scattered.
Dear nonara My language was a bit harsh. And I'm guessing it probably has some impact on running the translator as well (which sounds like an excuse).
And I'm not talking about "unreliable". When I accepted and understood your story. the code you gave me was taken out of context. Perhaps you misunderstood my purpose. I know the code is bad, and I know it's a workaround, because I wrote it. Nevertheless, I tried to get the existing test cases to pass and with the exception of ts-node, the rest of them passed on TS@5 and above. I also took your advice and fixed the issue with the version checking I was doing in that part. When I said "Please be precise and specific At least that the test case is broken.", I meant to say that if there is a problematic transformer, please let me know about it. As for the "5 minutes ago" comment, I don't know where you got that number from, but it's certainly a shorter period of time than yours, and the mitigation was inadequate. So from a troubleshooting perspective, my approach may seem pathetic and stupid. I'll leave it at that. PS. The comment in the link below are rude and show hostility. I guess I got carried away. |
@sunrabbit123 I appreciate that, and I don't think you're stupid. In earlier conversation, I tried to be kind, but my patience wore thin, when what I was saying was not accepted and was met with some hostility. That being said, I apologize also for anything I wrote which may have demonstrated irritability. The one thing on this is that in some messages you're claiming that this is a temporary solution that works in some cases. I told you that was fine, so long as it was made clear to users. In others places, however, you presented it as "competition" and a working solution. My main concern was that people understood this wasn't going to work as it's intended. As a maintainer in this space trying to unify and solve the issue of lagging support in the other library, seeing my code with a very important line broken being shipped and attempted to be merged into major libraries as a replacement library was concerning to me. In any event, thank you again for your message, and know that I don't think you're stupid or that your solution is "pathetic". All I wanted to do was to help you to understand that there was more to the problem. Best, PS. In respect for the others being emailed on this personal conversation, you're welcome to email me if you wanted to respond. |
I'm guessing it's a mix of contexts that led to the miscommunication. Also, I feel bad for the ts-jest developers for bringing this up here. I don't think it's good for misunderstandings to build up, so I'll just summarize my side of the story.
|
@sunrabbit123 @nonara thank you both for bringing the conversation back to a respectable tone. It's easy for conversations to get out of hand especially in a remote/online context so I want to commend you both for the way you've brought tempers back down. Thank you. I've gone through the comments here and elsewhere to gain a little bit of an understanding based on which, it sounds like the updates in the newly proposed library won't work when tsc is updated to v5. I've only read through these once so if I'm mistaken, please let me know. However, if this understanding is correct, I propose closing this PR for now. |
While this works for now, it may become unworkable in the future as the TS structure changes. Since you probably don't want this, let's close it. |
@kulshekhar Just a heads up — tsp v3 is now released and should work here. You can simply use |
Summary
Less frequent updates to ttypescript and less attention to maintenance.
That's why we recommend the following libraries
ttsc
And this support TS@5
Test plan
With node
@16.19.1
Does this PR introduce a breaking change?
Other information