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

Not working on TypeScript v5.5 update #152

Closed
samchon opened this issue Apr 27, 2024 · 8 comments
Closed

Not working on TypeScript v5.5 update #152

samchon opened this issue Apr 27, 2024 · 8 comments

Comments

@samchon
Copy link
Contributor

samchon commented Apr 27, 2024

Here is the reproducible repo, and you can see that ts-patch is not working on the [email protected].

@nonara
Copy link
Owner

nonara commented May 1, 2024

Hey @samchon! Thanks for the report. Will check it out

@nonara
Copy link
Owner

nonara commented May 3, 2024

They made some pretty interesting changes. I had to add a new slicer and update some other logic, but I believe I have it working.

There is just one test failing now, which seems to deal with esm only

   Command failed: node run-transform.js ts
    file:///C:/Users/ron/AppData/Local/Temp/tsp/esm/f7dfb328032bdd6492d9f6eebfbacbfd.mjs:2
    ReferenceError: exports is not defined

From what I've seen, it looks like the new ts doesn't always actually have a ts namespace. They seem to be doing some dynamic logic to prepare for converting to / handling ESM. I saw some discussion about it on Twitter, but I didn't follow too closely.

To accommodate that change, I added this to our patch source, and I call it whenever we need to find what used to be the ts namespace.

image

This seems to work in most cases, but looks like maybe not in the one, assuming that's what the error is referring to.

It's late here, so I'm going to call it a night, but if you want to take a look here is the PR:

I'm hoping the issue will be fairly easy to figure out. It may be as simple as just needing to expand the getTsInstance() function to accommodate whatever it should be in the case of its "esm mode".

Detail & Notes

Note to self:
I'm not sure if maybe we no longer need to use the esm package and wrap require in this case — if they finished full esm support, we can probably add in some logic where we only use the esm library if we need to. If memory serves, we may skip waiting for require throw if we know the file extension is esm ahead of time.

@nonara
Copy link
Owner

nonara commented May 3, 2024

I also found this nasty issue, which seems to affect even older versions of typescript on Node 21+

I think this is an actual bug in Node, as the error does not make sense, especially given the diagnostic data.

@nonara
Copy link
Owner

nonara commented Jun 3, 2024

Updated and working in v3.2.0

@samchon Let me know if you have any issues!

@nonara nonara closed this as completed Jun 3, 2024
@samchon
Copy link
Contributor Author

samchon commented Jun 4, 2024

https://github.com/samchon/typia/actions/runs/9369301300/job/25793460995

Tested, and checked it works properly on the TypeScript v5.5 beta.

Thanks for rapid updating.

@GulgDev
Copy link

GulgDev commented Aug 18, 2024

I'm still getting this issue on Node v22.5.1 and TypeScript 5.5.4 with ts-patch 3.2.1

@GulgDev
Copy link

GulgDev commented Aug 18, 2024

D:\sandbox\TypiaTest\node_modules\typescript\lib\tsc.js:135
                        const res = getEsmLibrary()(newModule)(targetFilePath);
                                                              ^
TypeError: Function.prototype.apply was called on undefined, which is a undefined and not a function
    at Module.wrappedRequire (D:\sandbox\TypiaTest\node_modules\typescript\lib\tsc.js:135:63)
    at require (node:internal/modules/helpers:123:16)
    at loadEntryFile (D:\sandbox\TypiaTest\node_modules\typescript\lib\tsc.js:401:31)
    at TspPlugin.createFactory (D:\sandbox\TypiaTest\node_modules\typescript\lib\tsc.js:379:44)
    at PluginCreator.createSourceTransformers (D:\sandbox\TypiaTest\node_modules\typescript\lib\tsc.js:270:56)

@GulgDev
Copy link

GulgDev commented Aug 18, 2024

Fixed by removing "type": "module" from package.json

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants