-
Notifications
You must be signed in to change notification settings - Fork 196
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
Fixed TypeScript linenumbers on server stacktraces #1509
Conversation
Some additional miscellaneous cleanup as well
Is this eslint dependency conflict common? I don't see it relating to any of the packages I have installed. |
Hey @SebastianRuan, saw that the failing e2e tests were getting in your way here, but I've hopefully got them fixed in #1510 If that gets merged first, you can probably merge mainline into this and see some green action :) |
@@ -1,6 +1,7 @@ | |||
import boolean from "boolean"; | |||
const devMode = boolean(get('DEV_MODE')); | |||
|
|||
/* Do NOT use source-map-support in production as it uses the non-standard stack property of Errors */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems problematic. Does this mean that stack traces will be impossible to follow in production? Is there a way around this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was worried about this warning here :
Non-standard: This feature is non-standard and is not on a standards track. Do not use it on production sites facing the Web: it will not work for every user. There may also be large incompatibilities between implementations and the behavior may change in the future.
source-map-support uses the Error.prototype.stack to make the translation between JS and TS. Although, Node.js does support Error.prototype.stack so I can enable it for production too.
Let's get this PR up to date and test it out, if it turns out to be small effort let's get this through this milestone. |
Closing in favor of #1685 |
Resolves #1382