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

V3 #96

Merged
merged 55 commits into from
Jun 13, 2023
Merged

V3 #96

merged 55 commits into from
Jun 13, 2023

Conversation

nonara
Copy link
Owner

@nonara nonara commented Apr 10, 2023

samchon and others added 29 commits April 7, 2023 20:01
- Installer and patch rewritten and compile
- Not fully tested
- Needs tests
- Set cache dir to node_modules/.cache convention
- Fixed almost all open issues
- Got patch working
- Got tspc working
- Fixed broken .editorconfig
- Fixed cache
- Speed optimizations
- Added "live" modules (tts replacement)
- Added perf testing script
- Fixed bugs
@nonara
Copy link
Owner Author

nonara commented Apr 10, 2023

Responding to @jakebailey:

Oh wow, to be clear, this branch is taking the same approach as prettier and using the // src/compiler/... comments as a region marker to try and apply patches? Hope we never move anything, or stop using esbuild... I almost guaranteet that the order of these modules in the output bundle is going to change in TS 5.1 just through refactors, not to mention that I am hoping to use ESM for our executables (like tsc.js), which will end up looking quite different.

Have you considered taking yarn's approach to patching, and instead fork TS, make the changes in-tree, then diff the output packages to produce a patch? I feel like that would end up safer... and also maybe provide a baseline for a PR into TS itself if we want to eventually accept plugins upstream.

I definitely understand where you're coming from. I agree that this would been the ideal route to go for simply maintaining what we were doing. I chose the current approach in view of the upcoming next version of tsp, which will be much more versatile, essentially allowing multi-faceted plugin packages. The changes in the latest TS sort of forcibly incentivized me to write the foundation for that sooner.

The short version is, you'll be able to bundle language service plugins and transformers in addition to two new features, which are middleware "hooks" and compiler transformers. The latter are applied at patch time on the compiler code itself. This should open up some unique opportunities, which I'm excited to explore.

For that reason, we needed to find an approach that made the most sense and could perform well. Having clearly defined boundaries via comments made this much more realizable, though I understand they're not guaranteed for future versions — more on that below.

What we have now are "slicers" and "patchers" associated with each TS version. Slicers run through and identify common points that we need to know (ie, file header, body wrapper start, body header, etc.)

These points are then used with memoizing so that we only ever create a SourceFile for the section of code that we know we need to transform. This approach had tremendous perf gains over simply creating an initial SourceFile.

Notably, we're also using caching, so most patching operations (after first run of a config state) should be faster than a yarn patch.

One disclaimer I should say about the current branch is that the organization is still not what I'd like it to be. It's a little messy and what's there is still largely an artifact of the exploratory process. It will be much cleaner, better organized, and its components more clearly defined as we move forward. This is a sort of alpha structure for what will come, that resembles something like what will be under the hood of the next.

I almost guarantee that the order of these modules in the output bundle is going to change in TS 5.1

Fortunately, the order won't matter. Slicers create a basic file map, keyed by filename.

not to mention that I am hoping to use ESM for our executables

With regard to this or any other potential changes, I definitely recognize that this is all subject to change. There is always some level of that which is to be expected. With that in mind, I crafted the approach to minimize as much potential of it as possible, while also aiming to make it easy to accommodate future versions if and when changes do happen.

Although current file structure doesn't reflect it, the idea is to likely make new sets of slicers and patchers on an as-needed basis. Mostly, we'll just need to update a few transformers if things change and if something moves, we tell it to look in a different SourceFile.

Overall, I think the gains in performance and targetability to original source files are worth the risk of changing tracking for a moved function from time to time. That said, users will have the option to apply transformers at a global SourceFile level, if they prefer.

We'll also want to have CI keep up with dev builds on a specified interval so we can stay ahead of changes.

One thing that would be great, though, would be if we could maintain the output of comment headers for the compiled files in the built TS libraries. Having that guarantee on those boundaries brings a major performance boost and would be more reliable than the previous method of using regex search for a particular declaration, block, etc.

If your side is open to that, I would be happy to contribute to ensure that's done for any future changes to the build process (like if you switch from esbuild), so it doesn't put any added burden on the TS team.

Any other thoughts you have, I welcome the discussion, and I appreciate your taking the time to weigh in as well as to let me know about the previous conflicts ahead of time!

- Circular program transformer
- added esModuleInterop by default
- Fixed issues with require & improved logic
@bennypowers
Copy link

@nonara
Copy link
Owner Author

nonara commented Apr 21, 2023

@bennypowers can you make sure that it isn't rc2? ts-patch -v

Beta3 should be stable, but rc2 has that issue.

@bennypowers
Copy link

Thanks @nonara for the response. Inspecting the npm package I see that rc2 is a later version than 3.0.0-beta3, and I had specified ^3.0.0-beta3 in my package.json. Removing the caret, rimraffing node_modules and package-lock, and reinstalling got me beta3 installed, and it does indeed build.

Thank you again for the response, and for your hard work on this upcoming release.

@mastodon0
Copy link

Got the same heap overflow issue with rc2 that is fixed by using beta3. Please let us know if you'd like us to test it again with a newer version.
Thank you for making it easy to switch vom ttsc <3

@nonara
Copy link
Owner Author

nonara commented Apr 27, 2023

Happy to help! I'll be fixing it and doing full release soon. Been going nonstop with work obligations for weeks now. Should be back to normal after next week.

Meanwhile beta3 should work.

@samchon
Copy link
Sponsor Contributor

samchon commented May 16, 2023

Can I ask how you are these days?

@bradennapier
Copy link

Added PR to fix current console bug: #100

@nonara
Copy link
Owner Author

nonara commented Jun 9, 2023

I finally had a few days to sit down and dedicate toward this. The current version should be working in all scenarios.

I'm including some notes here, if anyone is interested / for posterity.

Issues

Long story short, there were several big issues that made this more complex than it appeared.

The top things that contributed to this was my trying to drop the need for ts-node and to implement ESM support.

The following are a few details on the challenges

esm

This library has wild wiring. It runs everything in a separate virtual context and patches things in unusual ways. What it's doing is very smart, however. The author put a lot of thought into how to solve the problem very well.

With that said, it doesn't provide an easy was to simply compile code directly. It needs to be passed an actual file on the HD. Given more time, I could have worked out the wiring better and submitted a PR, but the current approach which uses temp files works well enough.

Another challenge here is it appeared that I couldn't simply register ts-node to compile first, due to the order we needed to register, so we needed to do some special wiring within our own require hook.

ts-node

In the last rc (rc3), weopted to use the compiler API directly, which let us drop dependency on ts-node. However, there were a lot of edge cases like properly dealing with package indexes, etc.

After some deeper investigation into ts-node and other major libraries which hook require in a major way, I learned that the primary method to handle it is to copy NodeJS require code and build on top of it. This gets very involved.

With that in mind, I needed to make the decision on if I wanted to replicate all of that and maintain it as node versions change. I concluded that it really doesn't make sense to do this. In addition to this portion, it turns out that we'd essentially have to replicate most of what is in ts-node.

It doesn't make sense as of this point to replicate it, but perhaps in a later major version it will.

tsconfig-paths

In the last rc, we had eliminated the need for this.

We could likely have easily kept it as such in this release. However, I looked through the code, and I noticed some other edge cases, like filtering out native libraries, etc.

Ultimately, this is another package that has had years to become solid. In my opinion, at this point, it makes more sense to simply rely on it, should users want this functionality.

Changes

ESM support now fully works. It throws if you don't have the esm package, prompting to install it.

Path alias mapping works, when you add resolvePathAliases: true to the plugin config. It will throw if you don't have tsconfig-paths.

Likewise, if you try to use a TS plugin, it will throw if you don't have ts-node available.

We are still hooking require, but in a more sound manner that layers well with all other hooks and only applies for the duration they're needed, which prevents cross-contamination with multiple plugins

@nonara
Copy link
Owner Author

nonara commented Jun 9, 2023

@samchon Doing well, thanks. I tested this with typia and it appears to work. I'm not doing a beta / rc release for this one, so if you'd like to check, you can clone this branch and give it a try. Let me know if you have any issues or comments.

I will be releasing full version directly, but I'm going to build out tests and make sure it's fully covered first.

samchon added a commit to samchon/typia that referenced this pull request Jun 10, 2023
samchon added a commit to samchon/nestia that referenced this pull request Jun 10, 2023
@samchon
Copy link
Sponsor Contributor

samchon commented Jun 10, 2023

@nonara Thanks for your dedications.

I'd tested in both typia and nestia.

As you can see from above github actions, there had not been any problem.

@nonara nonara merged commit cd69c1c into master Jun 13, 2023
6 checks passed
@nonara nonara deleted the v3-beta branch June 13, 2023 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants