Skip to content

chore(ssr): use subpath imports for estree validators #5102

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

Closed
wants to merge 5 commits into from
Closed
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
6 changes: 6 additions & 0 deletions eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,12 @@ export default tseslint.config(
message:
'Do not directly import runtime flags from @lwc/features. Use the global lwcRuntimeFlags variable instead.',
},
{
name: 'estree-toolkit',
importNames: ['is'],
message:
'Do not import `is` from estree-toolkit directly. Import from #estree/validators instead.',
},
],
'no-restricted-properties': [
'error',
Expand Down
3 changes: 3 additions & 0 deletions packages/@lwc/ssr-compiler/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@
}
}
},
"imports": {
"#estree/*": "./src/estree/*.js"
},
Copy link
Contributor

Choose a reason for hiding this comment

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

I would avoid package imports/exports. We've had issues in the past with Rollup/Jest and how it consumes exports.

"dependencies": {
"@babel/types": "7.26.3",
"@lwc/shared": "8.12.3",
Expand Down
10 changes: 3 additions & 7 deletions packages/@lwc/ssr-compiler/src/__tests__/estemplate.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,11 @@
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
*/

import { is, builders as b } from 'estree-toolkit';
import { builders as b } from 'estree-toolkit';
import { describe, test, expect } from 'vitest';
import { esTemplate, esTemplateWithYield } from '../estemplate';
import type { ClassDeclaration, FunctionDeclaration } from 'estree';

if (process.env.NODE_ENV !== 'production') {
// vitest seems to bypass the modifications we do in src/estree/validators.ts 🤷
(is.identifier as any).__debugName = 'identifier';
}
import { is } from '#estree/validators';

describe.each(
Object.entries({
Expand Down Expand Up @@ -61,7 +57,7 @@ describe.each(
`;
const doReplacement = () => tmpl(b.literal('I am not an identifier') as any);
expect(doReplacement).toThrow(
'Validation failed for templated node. Expected type identifier, but received Literal.'
/Validation failed for templated node. Expected type identifier, but received Literal./
);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@
*/

import { parse as pathParse } from 'node:path';
import { is, builders as b } from 'estree-toolkit';
import { esTemplate } from '../estemplate';
import { builders as b } from 'estree-toolkit';
import { bImportDeclaration } from '../estree/builders';
import { esTemplate } from '../estemplate';
import { bWireAdaptersPlumbing } from './wire';

import type { Program, Statement, IfStatement } from 'estree';
import type { ComponentMetaState } from './types';
import { is } from '#estree/validators';

const bGenerateMarkup = esTemplate`
const publicFields = new Set(${/*public fields*/ is.arrayExpression});
Expand Down
3 changes: 2 additions & 1 deletion packages/@lwc/ssr-compiler/src/compile-js/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import { generate } from 'astring';
import { traverse, builders as b, is } from 'estree-toolkit';
import { traverse, builders as b } from 'estree-toolkit';
import { parseModule } from 'meriyah';

import { DecoratorErrors } from '@lwc/errors';
Expand All @@ -28,6 +28,7 @@ import type {
} from 'estree';
import type { Visitors, ComponentMetaState } from './types';
import type { CompilationMode } from '@lwc/shared';
import { is } from '#estree/validators';

const visitors: Visitors = {
$: { scope: true },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
*/

import { is } from 'estree-toolkit';
import { generateScopeTokens } from '@lwc/template-compiler';
import { builders as b } from 'estree-toolkit/dist/builders';
import { builders as b } from 'estree-toolkit';
import { esTemplate } from '../estemplate';
import type { ExpressionStatement, Program, VariableDeclaration } from 'estree';
import { is } from '#estree/validators';

const bStylesheetTokenDeclaration = esTemplate`
const stylesheetScopeToken = '${is.literal}';
Expand Down
3 changes: 2 additions & 1 deletion packages/@lwc/ssr-compiler/src/compile-js/wire.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
*/

import { is, builders as b } from 'estree-toolkit';
import { builders as b } from 'estree-toolkit';
import { produce } from 'immer';
import { DecoratorErrors } from '@lwc/errors';
import { esTemplate } from '../estemplate';
Expand All @@ -24,6 +24,7 @@ import type {
BlockStatement,
} from 'estree';
import type { ComponentMetaState, WireAdapter } from './types';
import { is } from '#estree/validators';

interface NoSpreadObjectExpression extends Omit<ObjectExpression, 'properties'> {
properties: Property[];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
*/
import { builders as b } from 'estree-toolkit/dist/builders';
import { is } from 'estree-toolkit';
import { esTemplate, esTemplateWithYield } from '../estemplate';
import { isLiteral } from './shared';
import { expressionIrToEs } from './expression';
Expand All @@ -16,6 +15,7 @@ import type {
} from 'estree';
import type { TransformerContext } from './types';
import type { Node as IrNode, Text as IrText, Comment as IrComment } from '@lwc/template-compiler';
import { is } from '#estree/validators';

const bNormalizeTextContent = esTemplate`
normalizeTextContent(${/* string value */ is.expression});
Expand Down
3 changes: 2 additions & 1 deletion packages/@lwc/ssr-compiler/src/compile-template/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import { generate } from 'astring';
import { is, builders as b } from 'estree-toolkit';
import { builders as b } from 'estree-toolkit';
import { parse, type Config as TemplateCompilerConfig } from '@lwc/template-compiler';
import { DiagnosticLevel } from '@lwc/errors';
import { esTemplate } from '../estemplate';
Expand All @@ -17,6 +17,7 @@ import { optimizeAdjacentYieldStmts } from './shared';
import { templateIrToEsTree } from './ir-to-es';
import type { ExportDefaultDeclaration as EsExportDefaultDeclaration } from 'estree';
import type { CompilationMode } from '@lwc/shared';
import { is } from '#estree/validators';

// TODO [#4663]: Render mode mismatch between template and compiler should throw.
const bExportTemplate = esTemplate`
Expand Down
4 changes: 2 additions & 2 deletions packages/@lwc/ssr-compiler/src/compile-template/ir-to-es.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import { inspect } from 'util';

import { is, builders as b } from 'estree-toolkit';
import { builders as b } from 'estree-toolkit';
import { esTemplate } from '../estemplate';
import { Comment } from './transformers/comment';
import { Component, LwcComponent } from './transformers/component';
Expand All @@ -26,7 +26,7 @@ import type {
} from '@lwc/template-compiler';
import type { Statement as EsStatement, ThrowStatement as EsThrowStatement } from 'estree';
import type { TemplateOpts, Transformer, TransformerContext } from './types';

import { is } from '#estree/validators';
const bThrowError = esTemplate`
throw new Error(${is.literal});
`<EsThrowStatement>;
Expand Down
3 changes: 2 additions & 1 deletion packages/@lwc/ssr-compiler/src/compile-template/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
*/

import { builders as b, is } from 'estree-toolkit';
import { builders as b } from 'estree-toolkit';
import { normalizeStyleAttributeValue, StringReplace, StringTrim } from '@lwc/shared';
import { isValidES3Identifier } from '@babel/types';
import { expressionIrToEs } from './expression';
Expand All @@ -28,6 +28,7 @@ import type {
Expression as IrExpression,
Literal as IrLiteral,
} from '@lwc/template-compiler';
import { is } from '#estree/validators';

export function optimizeAdjacentYieldStmts(statements: EsStatement[]): EsStatement[] {
let prevStmt: EsStatement | null = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
*/

import { builders as b, is } from 'estree-toolkit';
import { builders as b } from 'estree-toolkit';

import { kebabcaseToCamelcase, toPropertyName } from '@lwc/template-compiler';
import { getChildAttrsOrProps } from '../../shared';
import { esTemplateWithYield } from '../../../estemplate';
Expand All @@ -14,6 +15,7 @@ import { getSlottedContent } from './slotted-content';
import type { BlockStatement as EsBlockStatement } from 'estree';
import type { Component as IrComponent } from '@lwc/template-compiler';
import type { Transformer } from '../../types';
import { is } from '#estree/validators';

const bYieldFromChildGenerator = esTemplateWithYield`
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
* SPDX-License-Identifier: MIT
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
*/
import { is } from 'estree-toolkit';
import { isUndefined } from '@lwc/shared';
import { expressionIrToEs } from '../../expression';
import { esTemplateWithYield } from '../../../estemplate';
Expand All @@ -16,6 +15,7 @@ import type {
Expression as IrExpression,
} from '@lwc/template-compiler';
import type { Statement as EsStatement } from 'estree';
import { is } from '#estree/validators';

const bYieldFromDynamicComponentConstructorGenerator = esTemplateWithYield`
const Ctor = '${/* lwcIs attribute value */ is.expression}';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import { produce } from 'immer';
import { builders as b, is } from 'estree-toolkit';
import { builders as b } from 'estree-toolkit';
import { bAttributeValue, optimizeAdjacentYieldStmts } from '../../shared';
import { esTemplate, esTemplateWithYield } from '../../../estemplate';
import { irChildrenToEs, irToEs } from '../../ir-to-es';
Expand Down Expand Up @@ -35,6 +35,7 @@ import type {
Slot as IrSlot,
} from '@lwc/template-compiler';
import type { TransformerContext } from '../../types';
import { is } from '#estree/validators';

const bGenerateSlottedContent = esTemplateWithYield`
const shadowSlottedContent = ${/* hasShadowSlottedContent */ is.literal}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
*/

import { builders as b, is } from 'estree-toolkit';
import { builders as b } from 'estree-toolkit';
import {
HTML_NAMESPACE,
isBooleanAttribute,
Expand Down Expand Up @@ -36,6 +36,7 @@ import type {
IfStatement as EsIfStatement,
} from 'estree';
import type { Transformer, TransformerContext } from '../types';
import { is } from '#estree/validators';

const bYield = (expr: EsExpression) => b.expressionStatement(b.yieldExpression(expr));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,15 @@
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
*/

import { builders as b, is } from 'estree-toolkit';
import { builders as b } from 'estree-toolkit';
import { esTemplate } from '../../estemplate';
import { irChildrenToEs } from '../ir-to-es';
import { getScopedExpression, optimizeAdjacentYieldStmts } from '../shared';

import type { ForEach as IrForEach } from '@lwc/template-compiler';
import type { Expression as EsExpression, ForOfStatement as EsForOfStatement } from 'estree';
import type { Transformer } from '../types';
import { is } from '#estree/validators';

const bForOfYieldFrom = esTemplate`
for (let [${is.identifier}, ${is.identifier}] of Object.entries(${is.expression} ?? {})) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
*/

import { builders as b, is } from 'estree-toolkit';
import { builders as b } from 'estree-toolkit';
import { esTemplate } from '../../estemplate';
import { irChildrenToEs } from '../ir-to-es';
import { optimizeAdjacentYieldStmts } from '../shared';
Expand All @@ -18,6 +18,7 @@ import type {
MemberExpression as EsMemberExpression,
} from 'estree';
import type { Transformer } from '../types';
import { is } from '#estree/validators';

function getRootMemberExpression(node: EsMemberExpression): EsMemberExpression {
return node.object.type === 'MemberExpression' ? getRootMemberExpression(node.object) : node;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
*/

import { is, builders as b } from 'estree-toolkit';
import { builders as b } from 'estree-toolkit';

import { esTemplateWithYield } from '../../estemplate';

Expand All @@ -20,6 +20,7 @@ import type {
Expression as EsExpression,
} from 'estree';
import type { Transformer } from '../types';
import { is } from '#estree/validators';

const bConditionalSlot = esTemplateWithYield`
if (isLightDom) {
Expand Down
19 changes: 10 additions & 9 deletions packages/@lwc/ssr-compiler/src/estemplate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,11 @@ import type {
Statement as EsStatement,
} from 'estree';
import type { Checker } from 'estree-toolkit/dist/generated/is-type';
import type { Validator } from './estree/validators';

/** Placeholder value to use to opt out of validation. */
const NO_VALIDATION = false;

/** A function that accepts a node and checks that it is a particular type of node. */
type Validator<T extends EsNode | null = EsNode | null> = (
node: EsNode | null | undefined
) => node is T;

/**
* A pointer to a previous value in the template literal, indicating that the value should be re-used.
* @see {@linkcode esTemplate}
Expand Down Expand Up @@ -93,15 +89,20 @@ const getReplacementNode = (
: validateReplacement(replacementNode))
) {
const expectedType =
(validateReplacement as any).__debugName ||
validateReplacement.name ||
'(could not determine)';
validateReplacement.__debugName || validateReplacement.name || '(could not determine)';
const actualType = Array.isArray(replacementNode)
? `[${replacementNode.map((n) => n && n.type).join(', ')}]`
: replacementNode?.type;
throw new Error(
const error = new Error(
`Validation failed for templated node. Expected type ${expectedType}, but received ${actualType}.`
);

if (validateReplacement?.__stack) {
error.message += `\n${validateReplacement.__stack.split('\n')[1]}`;
error.stack = validateReplacement.__stack;
}

throw error;
Comment on lines -96 to +105
Copy link
Contributor Author

Choose a reason for hiding this comment

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

With the type information, there's no need for as any.

}

return replacementNode;
Expand Down
30 changes: 26 additions & 4 deletions packages/@lwc/ssr-compiler/src/estree/validators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
*/

import { is } from 'estree-toolkit';
/* eslint-disable-next-line no-restricted-imports -- This is the only place where we should be importing `is` from estree-toolkit. */
import { is as _is } from 'estree-toolkit';

import { entries } from '@lwc/shared';
import type { Checker } from 'estree-toolkit/dist/generated/is-type';
import type { Node } from 'estree-toolkit/dist/helpers'; // estree's `Node` is not compatible?

import type { Node as EsNode } from 'estree';
/** A validator that returns `true` if the node is `null`. */
type NullableChecker<T extends Node> = (node: Node | null | undefined) => node is T | null;

Expand All @@ -26,9 +28,29 @@ export function isNullableOf<T extends Node>(validator: Checker<T>): NullableChe

isNullableOf.__debugName = 'isNullableOf';

/** A function that accepts a node and checks that it is a particular type of node. */
export type Validator<T extends EsNode | null = EsNode | null> = {
(node: EsNode | null | undefined): node is T;
__debugName?: string;
__stack?: string;
};

const is: {
[K in keyof typeof _is]: (typeof _is)[K] & { __debugName?: string; __stack?: string };
} = { ..._is };

if (process.env.NODE_ENV !== 'production') {
// Modifying another package's exports is a code smell!
for (const [key, val] of entries(is)) {
(val as any).__debugName = key;
val.__debugName = key;
Copy link
Collaborator

Choose a reason for hiding this comment

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

val here is still the function from estree-toolkit, so is.identifier imported via the actual package will still be modified. 😓

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True. I guess the only immediate win is making this compatible with esm, which admittedly isn't much for a debug-only feature.

Object.defineProperty(is, key, {
get: function get() {
const stack: { stack?: string } = {};
Error.captureStackTrace(stack, get);
val.__stack = stack.stack;
return val;
},
});
Comment on lines +44 to +52
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This feature, if deemed useful, can be added in a separate PR.

}
}

export { is };
Loading