-
Notifications
You must be signed in to change notification settings - Fork 14
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
malformed bundle is difficult to diagnose #801
Comments
I personally think insisting on a default export is problematic and I think we should work with named exports. Default exports in typescript are not the norm which in someway has something to do with them initially being difficult to implement before We don't appear to have any runtime checks and I don't think a generic error message is going to wash here. I created #530 which was refused and I do get the reasons but a generic error message with no indication that it is the default export will prove difficult to diagnose. |
I agree with you that in most cases a default export is not good, which we generally eschew it. However, in this case, not using a default export only sweeps the problem under the rug. We would need a "magic" named export that would have to be there, otherwise we couldn't discover and link the test. E.g. All that aside, in this case, the problem wasn't the default export, right? The test did have one, but it was the typescript config was missing and so the default export wasn't being bundled. I field the issue so that we could have a placeholder to remind us that sometimes things will be borked, and we want to give as much of a diagnosis as possible. |
I was forgetting about the reason why. That is tricky and would possibly need some AST trickery to cure it. The default export is good enough for now but we need really good validation that points to there not being a default export.
Oh maybe there is more than one case. This fails with the import { test } from '@bigtest/suite';
export const t = test("No default export"); That is why I am being pedantic about this, if I get the big long error message that points to the github issue then I probably will not get that this is a basic validation error message. I was thinking of adding something like this to if ( [test?.assertions, test?.children].some(c => {
return Array.isArray(c) === false;
})) {
throw new TestValidationError(`
Invalid Test: Test contains no assertions or children.
Does the test file ${test.path} contain a default export.
Test: ${[test.description].join(' → ')}`, test.path);
} The code in |
this will also obviously fail as there are no module.exports = {
description: 'no assertions or children';
} |
We should probably validate that there are both |
When encountering #800, this catestrophic misconfiguration only reported the realtively innocuous error message in the server output:
We should clearly indicate that bigtest could not process the bundle probably due to an internal error and that we should report the issue. It might even be panic-worthy.
The text was updated successfully, but these errors were encountered: