feat(compiler): script transformer experimentalErrorRecoveryMode

[rax-it]

Details

This change introduces a transformOption: experimentalErrorRecoveryMode which also sets babel parsers errorRecovery flag.
This changes lwc compilers, specifically babel-plugin-component behavior from

Throw at FIRST Error --> Collect as MANY errors as we can in a single invocation

While in theory we might still get errors in subsequent compile requests as some validation paths were skipped because we couldn't continue in a meaningful way.

These errors are then put in-to a bag of errors, {metadata: { lwcErrors: CompilerError[]}}, upon completion of the transform process our transformer looks for presence of these errors and throws an AggregateError.

Currently, only scriptTransformer is updated to utilize this behavior with other transformers (html, css) to follow suit.
experimentalErrorRecoveryMode defaults to false hence there should be no observable changes in current compilation scenarios.

Does this pull request introduce a breaking change?

• 😮‍💨 No, it does not introduce a breaking change.

Does this pull request introduce an observable change?

• 🤞 No, it does not introduce an observable change.

GUS work item

W-19992686

[wjhsf]

Suggested change

	}).toThrowAggregateError(['LWC1107: Invalid property name "dataInvalidProperty".']);
	}).toThrow(aggregatErrorMatcher(['LWC1107: Invalid property name "dataInvalidProperty".']));

We can use asymmetric matchers to avoid having to a whole new thing.

function aggregateErrorMatcher(messages: string[]) {
    return expect.objectContaining({
        name: 'AggregateError',
        message: 'Multiple errors occurred during compilation.',
        errors: messages.map((msg) => {
            return expect.objectContaining({
                name: 'Error',
                message: expect.stringContaining(msg),
            });
        }),
    });
}

[jye-sf]

What's the reasoning for putting this as its own separate lwcErrors vs the errors parameter?

[jye-sf]

Can you clarify what you mean by fallback assignment here?

[rax-it]

My bad, I missed to address this todo, updated!

[jye-sf]

See question above about this being a separate function. Feels like you could combine the code below with the code in the function and get cleaner conditional blocks?

[jye-sf]

Potentially even combine the code after the try/catch block here too.

[rax-it]

I think I was overthinking this a bit, simplified and updated

[jye-sf]

nit: I personally prefer having the return statement at the end even if it means result isn't a const. It forces a cleaner separation between the 'successful' codepath and the 'error' code paths, and doesn't hide the main successful return value in the middle.

[jye-sf]

Does this mean we don't want to try to aggregate any 'thrown' errors?

[rax-it]

This should only happen when babel.transformSync call throws, compiler-plugin errors should not cause the compilation itself to fail. This will be an instance of Error and we don't have our "lwcErrors" bag of errors , hence nothing to Aggregate this error to.

This also means that when running in errorRecoveryMode compiler can now throw 2 types of errors CompilerError and an AggregateError but I think that might me fine, unless we need to converge them to a single error type for some reason.
Let me know if you have any thoughts here?

[jye-sf]

Suggested change

	// Should only occurs in error recovery mode when config validation has already failed
	// Should only occur in error recovery mode when config validation has already failed

[jye-sf]

Also, what does this mean?

[rax-it]

This essentially means that we have reached a path that we shouldn't have reached given the existing constraints/validations in place.

Ex: we reach wire.transfrom only when we have successfully gathered metadata, now say if we failed to identify the decorator type, instead of throwing we just log that error and continue to parse but this causes an issue here where we are expecting wireConfig.get('properties') to be an Array but instead it could be anything like string, undefined.

So to prevent compilation from breaking unexpectedly we just return an skip processing these props in errorRecoveryMode since we know this path already has known issues and is going to fail.

[rax-it]

What's the reasoning for putting this as its own separate lwcErrors vs the errors parameter?

Metadata that gets passed around in babel doesn't have any keys , I just gave it an explicit name to avoid any conflict/confusion if we update metadata to have extra key/values :)