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

fix(common): update TS module resolution flow #1659

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

arturovt
Copy link
Collaborator

@arturovt arturovt commented Feb 4, 2024

This commit updates the implementation for resolving .ts files.
Instead of registering the ts-node project only once, we now refrain from
doing so since there might be multiple projects with different configurations.
The current approach involves dynamically switching the implementation for
registering and unregistering the project after the .ts file has been transpiled
and resolved. This change addresses an issue where warnings were encountered when
ts-node attempted to register with different configurations. The number of configurations
is no longer a concern, as each time we need to read a .ts file, a new TS project is
registered. This adjustment does not impact performance or other attributes because ts-node
allows native project disabling. Part of the implementation has been adapted from what Nrwl Nx
already has; we can find their implementation here:
https://github.com/nrwl/nx/blob/master/packages/nx/src/plugins/js/utils/register.ts
It's worth noting that their implementation is somewhat versatile, as it also supports SWC.

Closes: #1197
Closes: #1213
Closes: #1730

@arturovt arturovt force-pushed the fix/ts-resolution branch 2 times, most recently from 0de7266 to b2a36d8 Compare February 4, 2024 20:55
@arturovt arturovt marked this pull request as ready for review February 4, 2024 21:05
@just-jeb
Copy link
Owner

just-jeb commented Feb 5, 2024

@arturovt The CI is failing 😬.

@arturovt arturovt force-pushed the fix/ts-resolution branch 3 times, most recently from 6b5ca2f to 15c4e1b Compare March 10, 2024 21:51
@arturovt
Copy link
Collaborator Author

Ugh that CI is so weird with random failures... doesn't even show any error...

@arturovt arturovt force-pushed the fix/ts-resolution branch 5 times, most recently from f9ab45e to e02da5d Compare April 4, 2024 08:27
@arturovt
Copy link
Collaborator Author

arturovt commented Apr 4, 2024

@just-jeb could you run ci locally? Maybe you can see the actual error…

@just-jeb
Copy link
Owner

just-jeb commented Apr 4, 2024

@arturovt I'll give it a look. Were you unable to run it locally?

This commit updates the implementation for resolving `.ts` files.
Instead of registering the `ts-node` project only once, we now refrain from
doing so since there might be multiple projects with different configurations.
The current approach involves dynamically switching the implementation for
registering and unregistering the project after the `.ts` file has been transpiled
and resolved. This change addresses an issue where warnings were encountered when
`ts-node` attempted to register with different configurations. The number of configurations
is no longer a concern, as each time we need to read a `.ts` file, a new TS project is
registered. This adjustment does not impact performance or other attributes because `ts-node`
allows native project disabling. Part of the implementation has been adapted from what Nrwl Nx
already has; we can find their implementation here:
https://github.com/nrwl/nx/blob/master/packages/nx/src/plugins/js/utils/register.ts
It's worth noting that their implementation is somewhat versatile, as it also supports SWC.

Closes: #1197
Closes: #1213
Closes: #1730
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants