Skip to content
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

feat(ssr): compiler error location #5114

Merged
merged 1 commit into from
Jan 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 95 additions & 0 deletions packages/@lwc/ssr-compiler/src/__tests__/compilation.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,27 @@
import path from 'node:path';
import { describe, test, expect } from 'vitest';
import { CompilerError } from '@lwc/errors';
import { compileComponentForSSR } from '../index';

expect.addSnapshotSerializer({
test(val) {
return val instanceof CompilerError;
},
serialize(val: CompilerError, config, indentation, depth, refs, printer) {
return printer(
{
message: val.message,
location: val.location,
filename: val.filename,
},
config,
indentation,
depth,
refs
);
},
});

describe('component compilation', () => {
test('implicit templates imports do not use full file paths', () => {
const src = `
Expand Down Expand Up @@ -31,4 +51,79 @@ describe('component compilation', () => {
const { code } = compileComponentForSSR(src, filename, {});
expect(code).toContain('import tmpl from "./component.html"');
});

describe('wire decorator', () => {
test('error when using @wire and @track together', () => {
const src = `import { track, wire, LightningElement } from "lwc";
import { getFoo } from "data-service";
export default class Test extends LightningElement {
@track
@wire(getFoo, { key1: "$prop1", key2: ["fixed", "array"] })
wiredWithTrack;
}
`;
expect(() => compileComponentForSSR(src, 'test.js', {}))
.toThrowErrorMatchingInlineSnapshot(`
{
"filename": "test.js",
"location": {
"column": 2,
"length": 59,
"line": 5,
"start": 156,
},
"message": "LWC1095: @wire method or property cannot be used with @track",
}
`);
});
test('throws when wired method is combined with @api', () => {
const src = `import { api, wire, LightningElement } from "lwc";
import { getFoo } from "data-service";
export default class Test extends LightningElement {
@api
@wire(getFoo, { key1: "$prop1", key2: ["fixed"] })
wiredWithApi() {}
}
`;

expect(() => compileComponentForSSR(src, 'test.js', {}))
.toThrowErrorMatchingInlineSnapshot(`
{
"filename": "test.js",
"location": {
"column": 2,
"length": 50,
"line": 5,
"start": 152,
},
"message": "LWC1095: @wire method or property cannot be used with @api",
}
`);
});
test('throws when computed property is expression', () => {
const src = `import { wire, LightningElement } from "lwc";
import { getFoo } from "data-service";
const symbol = Symbol.for("key");
export default class Test extends LightningElement {
// accidentally an array expression = oops!
@wire(getFoo, { [[symbol]]: "$prop1", key2: ["fixed", "array"] })
wiredFoo;
}
`;

expect(() => compileComponentForSSR(src, 'test.js', {}))
.toThrowErrorMatchingInlineSnapshot(`
{
"filename": "test.js",
"location": {
"column": 2,
"length": 9,
"line": 7,
"start": 288,
},
"message": "LWC1200: Computed property in @wire config must be a constant or primitive literal.",
}
`);
});
});
});
24 changes: 20 additions & 4 deletions packages/@lwc/ssr-compiler/src/compile-js/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
* SPDX-License-Identifier: MIT
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
*/
import { generateErrorMessage, type LWCErrorInfo } from '@lwc/errors';
import { type LWCErrorInfo, generateCompilerError } from '@lwc/errors';
import type { Node } from 'estree';

// This type extracts the arguments in a string. Example: "Error {0} {1}" -> [string, string]
type ExtractArguments<
Expand All @@ -18,8 +19,23 @@ type ExtractArguments<
: Args; // No `N` found, nothing more to check

export function generateError<const T extends LWCErrorInfo>(
node: Node,
error: T,
...args: ExtractArguments<T['message']>
): Error {
return new Error(generateErrorMessage(error, args));
...messageArgs: ExtractArguments<T['message']>
) {
return generateCompilerError(error, {
messageArgs,
origin: node.loc
? {
filename: node.loc.source || undefined,
location: {
line: node.loc.start.line,
column: node.loc.start.column,
...(node.range
? { start: node.range[0], length: node.range[1] - node.range[0] }
: {}),
},
}
: undefined,
});
}
17 changes: 10 additions & 7 deletions packages/@lwc/ssr-compiler/src/compile-js/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -225,20 +225,20 @@ function validateUniqueDecorator(decorators: EsDecorator[]) {

const expressions = decorators.map(({ expression }) => expression);

const hasWire = expressions.some(
const wire = expressions.find(
(expr) => is.callExpression(expr) && is.identifier(expr.callee, { name: 'wire' })
);

const hasApi = expressions.some((expr) => is.identifier(expr, { name: 'api' }));
const api = expressions.find((expr) => is.identifier(expr, { name: 'api' }));

if (hasWire && hasApi) {
throw generateError(DecoratorErrors.CONFLICT_WITH_ANOTHER_DECORATOR, 'api');
if (wire && api) {
throw generateError(wire, DecoratorErrors.CONFLICT_WITH_ANOTHER_DECORATOR, 'api');
}

const hasTrack = expressions.some((expr) => is.identifier(expr, { name: 'track' }));
const track = expressions.find((expr) => is.identifier(expr, { name: 'track' }));

if ((hasWire || hasApi) && hasTrack) {
throw generateError(DecoratorErrors.CONFLICT_WITH_ANOTHER_DECORATOR, 'track');
if (wire && track) {
throw generateError(wire, DecoratorErrors.CONFLICT_WITH_ANOTHER_DECORATOR, 'track');
}
}

Expand All @@ -252,6 +252,9 @@ export default function compileJS(
let ast = parseModule(src, {
module: true,
next: true,
loc: true,
source: filename,
ranges: true,
Comment on lines +255 to +257
Copy link
Contributor Author

@cardoso cardoso Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple possible changes to avoid bloating AST:

  • It didn't look like ranges are used by the LSP/VSCode extension, I only included it in order to match babel-plugin-compiler.
  • Source/filename could be moved to the ComponentMetaState. That would make things a bit coupled, but it might be worth it and is what babel-plugin-compiler seems to do:
    function generateError(
    source: NodePath<types.Node>,
    { errorInfo, messageArgs }: DecoratorErrorOptions,
    state: LwcBabelPluginPass
    ) {
    const message = generateErrorMessage(errorInfo, messageArgs);
    const error = source.buildCodeFrameError(message);
    (error as any).filename = state.filename;
    (error as any).loc = normalizeLocation(source);
    (error as any).lwcCode = errorInfo && errorInfo.code;
    return error;
    }

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Matching babel-plugin-component exactly here is not necessary IMO, unless it's fulfilling some function in VS Code... and I take it it's not?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}) as EsProgram;

const state: ComponentMetaState = {
Expand Down
17 changes: 13 additions & 4 deletions packages/@lwc/ssr-compiler/src/compile-js/wire.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function getWireParams(
const { decorators } = node;

if (decorators.length > 1) {
throw generateError(DecoratorErrors.ONE_WIRE_DECORATOR_ALLOWED);
throw generateError(node, DecoratorErrors.ONE_WIRE_DECORATOR_ALLOWED);
}

// validate the parameters
Expand Down Expand Up @@ -94,7 +94,10 @@ function validateWireId(

// This is not the exact same validation done in @lwc/babel-plugin-component but it accomplishes the same thing
if (path.scope?.getBinding(wireAdapterVar)?.kind !== 'module') {
throw generateError(DecoratorErrors.COMPUTED_PROPERTY_MUST_BE_CONSTANT_OR_LITERAL);
throw generateError(
path.node!,
DecoratorErrors.COMPUTED_PROPERTY_MUST_BE_CONSTANT_OR_LITERAL
);
}
}

Expand Down Expand Up @@ -129,9 +132,15 @@ function validateWireConfig(
continue;
}
} else if (is.templateLiteral(key)) {
throw generateError(DecoratorErrors.COMPUTED_PROPERTY_CANNOT_BE_TEMPLATE_LITERAL);
throw generateError(
path.node!,
DecoratorErrors.COMPUTED_PROPERTY_CANNOT_BE_TEMPLATE_LITERAL
);
}
throw generateError(DecoratorErrors.COMPUTED_PROPERTY_MUST_BE_CONSTANT_OR_LITERAL);
throw generateError(
path.node!,
DecoratorErrors.COMPUTED_PROPERTY_MUST_BE_CONSTANT_OR_LITERAL
);
}
}

Expand Down