-
Notifications
You must be signed in to change notification settings - Fork 499
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
config does not play well with node-support-source-map #530
Comments
I have a hard time categorizing this as a bug. There is no documented expectation of this library working well with this other library. This sounds more like a feature request than a bug report. Anyone with .ts knowledge want to make a contribution for this issue? |
@lorenwest I don't say that Regards, |
Agreed - it would be a good addition/improvement. We have a few contributors that have added .ts support, and I'll mark this issue as a collaboration request. |
I've updated the wiki (because it actually let me...) to indicate that https://github.com/lorenwest/node-config/wiki/Configuration-Files#typescript---ts This issue is devious enough that I think it warrants it. |
Hey, I've submitted PR #721 with a possible fix. Tested locally on the demo repo https://github.com/PavelPolyakov/config-vs-node-source-map-support (after updating deps) and it seems to work on my end |
Also I think it might improve or even fix #613 given that it should prevent unnecessary processing of files other than Quick test results on https://github.com/lavagri/nestjs-config-test (after updating
So there seems to be significant improvement for A, B, C modules import (297 ms vs 769 ms total) |
I tried using @andrzej-woof's branch and I still had incorrect line reporting for failing Jest tests. The PR here seemed to fix the issue for me. require.extensions is deprecated and the require.extensions object does not even have the .ts key(It only has .js, .json, and .node as far as I can tell) so I am not sure the original intent of the removed code. In any case this seems to fix the incorrect line reporting issue on Jest test failures for my project so I figured I would test Cunningham's law and make a PR in hopes that someone more familiar with this library would save me a bit of code archeology. Node v16.13.1 |
Hi @EvanTedesco do you run your code via ts-node/ts-jest? I admit my PR won't solve your problem (as far as I understand it), but by removing this code I think you effectively disable loading any ts config files with compiled code (unsure if they're loaded at all even with ts-node. Of course I might be wrong, just sharing thoughts here |
A bit more context, after some further investigation, regarding As for running with |
Yes I am running with ts-node/ts-jest. Thank you for the elaboration/clarification. |
I'm submitting a ...
What is the current behavior?
Currently, if you store your configs using
ts
extension and load them usingconfig
-source-map-support
for node applications stops working. By not working I mean - there are no references to the originalts
files in the stack trace of the errors.I consider this functionality very important.
What is the expected behaviour?
They can work together and don't interfere each other (namely,
config
has no affect on how other modules work).Expected behaviour:
Please tell us about your environment:
Other information
Here is the repo where you can reproduce the issue:
https://github.com/PavelPolyakov/config-vs-node-source-map-support
The text was updated successfully, but these errors were encountered: