Misleading error message when passing an invalid stylesheet

[pmdartus]

Description

The LWC engine throws a cryptic error when it attempts to evaluate a stylesheet that hasn't been generated by the compiler.

Steps to Reproduce

Playground: link

In this example, the x/uPlot module contains both a JavaScript and a StyleSheet file. The x/uPlot module is referenced both in the x/chart JavaScript and StyleSheet files. The developer expected the JavaScript import resolves to the uPlot.js and the StyleSheet import resolves to the uPlot.css.

However, the LWC module resolver doesn't work this way. Importing a module containing both a JavaScript and a StyleSheet file would always resolve the JavaScript file. That said it's not obvious because the LWC engine would attempt to evaluate the JavaScript default export as a stylesheet resulting in a cryptic error.

Expected Results

Uncaught TypeError: Unexpected LWC stylesheet content.

Actual Results

Uncaught TypeError: Cannot read properties of undefined (reading 'mode')
    at uPlot (main.js:10148:23)
    at evaluateStylesheetsContent (main.js:3356:33)
    at getStylesheetsContent (main.js:3368:15)
    at main.js:5209:36

Browsers Affected

All

Version

• LWC: 2.30.0

Possible Solution

The LWC engine should do a brand-check of the stylesheet function before evaluating it. If the check fails, the engine should report a TypeError.

[git2gus]

This issue has been linked to a new work item: W-12040411

[nolanlawson]

Looks related/duplicate of #1887

[nolanlawson]

Related: #3122

[jye-sf]

@nolanlawson I'm looking at this bug and would love your input with your stylesheet expertise.

I can see three possible approaches to fix this issue for 244:

1. Fix in the compiler so we're not serializing the JS import into the CSS stylesheet and passing around an improperly constructed template.stylesheets.
   • This addresses the root but doesn't add any guards to the engine itself for invalid stylesheets.
2. Validate stylesheet during evaluateStylesheetsContent
   • Our style-compiler's serialize() uses STYLESHEET_IDENTIFIER with a token.
   • Is it safe to rely on this to validate that stylesheet.name.indexOf(STYLESHEET_IDENTIFIER) === 0 for valid stylesheets? Or am I missing edge cases?
   • I can't see anything else inherent in the function that would distinguish between a valid or an invalid stylesheet, but is there anything I'm missing?
3. try/catch when evaluating stylesheet content to write a clearer error message
   • This would rely on the JS default export doing something to cause an error.
   • Ideally, we would validate the stylesheet function without having to execute it, so not a great solution.

I'm leaning towards implementing the second solution in evaluateStylesheetContent but would like a sanity check.

[nolanlawson]

@jye-sf So first off, the repro from the playground above does not seem to throw any error (Unexpected LWC stylesheet content or otherwise). Also based on the component structure, I don't understand how there could be a module resolution confusion between uPlot and uPlotCss. They seem perfectly disambiguated in the repro. (@pmdartus Do you recall what might be going on here?)

As for the more generic problem of distinguishing LWC-compiled stylesheet functions from arbitrary functions-that-return-a-string (#3122), I figured we should follow the same pattern as freezeTemplate/registerTemplate to solve this. I.e. we should mark the stylesheet function in the compiler in a way where we know for sure that it was compiled by LWC. (Checking whether the function name is stylesheet is not really good enough... developers can define their own functions with that name. 🙂)

Also of course we would need to start with a warning first and only make it an error later, as we do with freezeTemplate.

[jye-sf]

The repro above looked like a workaround-ed version of the issue. I was able to reproduce the issue by copying the CSS into the uPlot module and importing from there. Here it is (also with an updated LWC version): https://stackblitz.com/edit/salesforce-lwc-2tgazw; sorry for not including it with my original question.

I think fully implementing the registerSpreadsheet mechanism per #3122 would help as a guard for the engine, but that by itself wouldn't address this specific issue. In this use case, both the real stylesheet and the arbitrary function are being put together by the compiler, so both would be signed if we don't disambiguate during compilation. The generic problem of distinguishing stylesheet functions from arbitrary functions would still need to be solved in the compiler/resolver before we can mark them correctly.

function uPlot(opts, data, then) { ... }
function stylesheet(token, useActualHostSelector, useNativeDirPseudoclass) { ... }

var _implicitStylesheets = [uPlot, stylesheet];

...

var _tmpl$1 = registerTemplate(tmpl$1);
tmpl$1.stylesheets = [];

if (_implicitStylesheets) {
  tmpl$1.stylesheets.push.apply(tmpl$1.stylesheets, _implicitStylesheets);
}
tmpl$1.stylesheetToken = "x-chart_chart";
freezeTemplate(tmpl$1);

[jye-sf]

I guess the question is whether this issue is urgent enough that we want a stopgap change to the error message until we can implement registerStylesheet and look into disambiguating the functions in the compiler/resolver. That does feel like the correct solution, but it's definitely more work than a simple guard in the engine (and likely won't make it into 244).

Personally, if the main issue with using the function name stylesheet is that it won't work for developers with components named stylesheet*, I think that's okay. Their experience would be no different from what we have now, while all the other developers' experiences with this error would be improved. My main concern would be whether using this indicator would filter out valid stylesheets and break valid use cases.

[nolanlawson]

OK, I think I understand now. The core issue is:

@import 'x/uPlot'; /* imports JS, not CSS */

I wouldn't say it's urgent, but it is annoying. I also think it would make the most sense to implement registerStylesheet first (since we want it anyway) before doing this feature.

I think fully implementing the registerSpreadsheet mechanism per #3122 would help as a guard for the engine, but that by itself wouldn't address this specific issue. In this use case, both the real stylesheet and the arbitrary function are being put together by the compiler, so both would be signed if we don't disambiguate during compilation.

I believe in this case uPlot.js would be compiled by our Babel compiler, whereas stylesheet.css would be compiled by our style compiler, correct? So we should be able to disambiguate them at the compiler level. (I.e. only one would get passed into registerStylesheet.)

[jye-sf]

You're right, I'm definitely overcomplicating things 😅. After a closer look at the compiler this weekend I see how we can easily pass only the stylesheet into registerStylesheet before the module resolution for the import happens.

[nolanlawson]

Fixed by #3443