Hide internal @babel/core functions in config errors#11554
Hide internal @babel/core functions in config errors#11554nicolo-ribaudo merged 13 commits intobabel:mainfrom
@babel/core functions in config errors#11554Conversation
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/52794/ |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 686dfa8:
|
686dfa8 to
b61826d
Compare
|
This PR looks great! The current stack provides little useful information. |
2fb805c to
e69b896
Compare
e69b896 to
655a4d9
Compare
651e21b to
b6af783
Compare
77cd490 to
72216af
Compare
72216af to
fae9608
Compare
14a3300 to
a75ea8b
Compare
|
@liuxingbaoyu Since you use windows, when you have time could you clone this PR, run |
|
This is the same as #14671 (comment), by the way I also found a typo there although it works fine.😂 |
|
To be honest, the CI exception from the fork repo scares me.😅 |
Co-authored-by: liuxingbaoyu <30521560+liuxingbaoyu@users.noreply.github.com>
|
Thank you! 😄 |
| } | ||
|
|
||
| if (!options) throw new Error(`${filepath}: No config detected`); | ||
| if (!options) throw new ConfigError(`No config detected`, filepath); |
There was a problem hiding this comment.
Maybe there is some lint rule to make it a normal string, but in this PR I think it's ok.🤫
There was a problem hiding this comment.
There's @typescript-eslint/quotes, which can be configured with allowTemplateLiterals: false to avoid unnecessary template literals (despite the name, it does not forbid all usage of template literals). To be compatible with Prettier, avoidEscape: true must also be set (even if you extend eslint-config-prettier, individual rule configurations take precedence).
| function (this: any) { | ||
| setupPrepareStackTrace(); | ||
| return fn.apply(this, arguments); | ||
| } as any as Fn, |
There was a problem hiding this comment.
The type casting can be avoided if we parameterize the function's input and output.
export function beginHiddenCallStack<T, A extends any[], R>(
fn: (this: T, ...args: A) => R,
) {
if (!SUPPORTED) return fn;
return Object.defineProperty(
function (this: T, ...args: A) {
setupPrepareStackTrace();
return fn.apply(this, args);
},
"name",
{ value: STOP_HIDNG },
);
}There was a problem hiding this comment.
We still need to type cast somewhere, because the functions that we receive in the config have type Function which is not compatible with the "explicit" function types. However, I moved them to the config validation logic so that the "internals" are properly typed.
df5ef02 to
89542ad
Compare
| let newTrace = []; | ||
|
|
||
| const isExpected = expectedErrors.has(err); | ||
| let status = isExpected ? "hiding" : "unknown"; |
There was a problem hiding this comment.
Nit: Can you restrict the typing here? It seems to me status can only be "hiding" | "showing" | "unknown".
| * injcectVirtualStackFrame(error, filename) function: those are added on the top | ||
| * of the existig stack, after hiding the possibly hidden frames. | ||
| * In the example above, if we called injcectVirtualStackFrame(error, "h") on the | ||
| * expected error thrown by c(), it's shown call stack would have been "h, e, f". |
There was a problem hiding this comment.
| * expected error thrown by c(), it's shown call stack would have been "h, e, f". | |
| * expected error thrown by c(), its shown call stack would have been "h, e, f". |
May extend the example a bit, for example, what will be the shown call stack if we called injcectVirtualStackFrame(error, "h") and injcectVirtualStackFrame(error, "i")?
This PR introduces some logic to hide
@babel/coreinternals in error stack traces for errors coming from outside@babel/coreitself (for example, coming from config files or plugins/presets):@babel/coreintroduces a lot of frames (and all duplicated, because of how we use gensync), which are not really useful.Unexpected errors thrown inside
@babel/coreitself still have the complete stack trace, because they are bugs and we need debug info.If you want to see the old stack traces, you can replace
const SUPPORTED = !!Error.captureStackTrace;withconst SUPPORTED = false;and run the tests.