-
-
Notifications
You must be signed in to change notification settings - Fork 540
Sourcemap failing when filename has spaces or is otherwise affected by URL percent-encoding #1322
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
Comments
I haven't had a chance to investigate this issue or go through the issue with a fine-tooth comb, but thank you for including lots of detail; it will surely be useful. The following answer is incomplete but I hope is helpful. I see you're on Windows. I've seen issues in the past where windows paths were either not handles correctly by us, or third-party components returned I've also seen newline issues where we erroneously assumed Linux newlines and did some string manipulation wrong, which might be affecting how we manipulate the sourcemap comment to the end of the emitted We have sourcemap tests, but it looks like none use The code being executed by the tests is https://github.com/TypeStrong/ts-node/blob/main/tests/throw.ts It has a ton of whitespace at the top which will all be removed by compilation, which makes it easier to see what's going on. If the stack trace says the error happened on line 100, sourcemaps worked. If the line number is much lower, they likely failed entirely. Unrelated, but I also noticed those test files do not use any type syntax, so if compilation was erroneously skipped entirely, the code would still execute. I should add a type annotation to ensure that compilation occurs. |
Thanks for the detailed reply. Ithink it's also important to note that the source map is not exactly on the line with the error. The source map says The second one could make sense although I'm struggling to see how hard LF/CRLF is to manage in JavaScript, and where the issue might arise, but it's a possibility if you've encountered it before. If you haven't already by later when I get the chance, I'll have a look at those tests and try to reproduce the issue in a minimal version since I think that would be pretty useful (... I doubt you want to go through setting up mongodb for my project). |
Does this mean that when you tried to create a minimal reproduction, the problem went away? That would imply that there is a component of your project, removed in the process of trying to minimize the reproduction, that may be triggering the failure. I see in the compiled If you were to do the trick that we use in our tests, adding 100 or even 500 lines of whitespace to the beginning of the file, I bet you would see a much larger difference between the desired line number and what is reported in the stack trace. The stack trace would say line 83, but you would want it to say line 585. (due to the massive whitespace added to the file) |
I see also that you have a space in the filename. Perhaps spaces in sourcemap paths are handled in a way we don't expect. It is pretty easy to add additional debugging log statements to ts-node when you're trying to debug this. You can see that this is where we attempt to retrieve a sourcemap from memory. You can add This style of debugging is alien to a lot of developers, but in my experience it is pretty quick to do, and gives you lots of detailed, useful information. |
I've added tests for |
Are you sure that To test this, you can delete the compiled |
Thanks for the reply again, I have only looked at the project earlier today and if I'm completely honest, I had completely forgotten about this issue as I didn't really encounter it aside from my manual "trying to break it" by adding a random line that would error. You're right that I failed to make my reproductions have the same error. In regards to the trick that you use in your tests (adding the whitespace at the start of the file), you're right that the stacktrace remained at line I'll start debugging through the package itself now and get back on my results. As I began this while you posted your next 2 comments, I haven't looked into the sourcemap with the filenames since it seems as though that makes no difference, and I can confirm that |
Interesting, can you copy-paste the full error there? EDIT I mean the full log message, errors, everything. Include as much as you can. The screenshot is truncated, and console.log typically adds a space between arguments. However, in your screenshot, I see |
I hadn't realised how large the logs were, sorry. I've posted them here https://gist.github.com/edqx/ab5005e8deb26a22a2a2f387e2ec62c7 Also I specifically meant the last one where it seems to say |
Thanks, this is very helpful. This bit looks interesting:
ts-node needs to manipulate the For example, spaces in a URL are encoded as I'm working on another task at the moment so I'll come back to this later. But I think you may have found the root cause. |
In the 4.1.1 release They implemented microsoft/TypeScript#40951 It percent-encodes the |
"transpileOnly": true
Why didn't this occur in your tests which had spaces in the filename? |
I figured it out. Final conclusion: This is not related to This is already fixed on our main branch by #1160, even though the fix was not intentional. This may seem confusing but remember it is not related to transpile-only. #1160 changed the way we append Since the fix in #1160 was accidental, I've also implemented #1330 to make our internal logic understand percent-encoded This fix will be released in v10 which I hope to release this weekend. In the meantime you can install directly from git: I'm going to close this issue since all necessary work has been completed. Thank you again @edqx for your assistance locating the underlying issue. |
Already waiting for v10 for the |
EDIT changed the title to describe the problem discovered after investigation. Old title was "Invalid line number using
"transpileOnly": true
" - @cspotcodeExpected Behavior
Error stacktrace should show the exact, correct line number when using
"transpileOnly"
.Actual Behavior
It doesn't, the line number is off by around
8
, although I imagine this changes.Understandably, it is the same line or close to the line that appears in the compiled JS file, except that the file ends in
.ts
The file can be seen here:
https://github.com/edqx/swagclan/blob/ee46636fcb42ce5c82c596861bab5b8ab963605b/src/api/routes/commands/POST%20index.ts#L77
And here is the compiled file:
https://gist.github.com/edqx/8bbc01d58ee0f759b09984920cd15bc3
I'm unable to create a minimal reproduction, currently I'm only looking for advice if this issue has been stumbled upon before, through searching the issues, I found #1055 but that issue has since been closed and did not fix my issue. I will, however, try to create one if really required, although I hope I have provided enough information as it is.
Specifications
ts-node v9.1.1
node v14.13.0
compiler v4.2.4
Windows 10.0.19041 Build 19041
The text was updated successfully, but these errors were encountered: