Skip to content

Commit

Permalink
chore: leverage the full power of @typescript-eslint @W-15024672 (#3995)
Browse files Browse the repository at this point in the history
* chore(scripts): add typecheck script

* ci: add step to run type checks

* Revert "ci: add step to run type checks"

This reverts commit 9c81472.

* Revert "chore(scripts): add typecheck script"

This reverts commit f441ba3.

* chore: remove redundant rules

they're included in recommended rulesets

* chore(eslint): use rules from @typescript-eslint/recommended

disable no-explicit-any and ban-ts-comment because there are a lot of violations.

* chore(eslint): report (and remove) unused disable directives

* chore(eslint): add recommended type checked rules

disable most that are related to `any`.

* chore(eslint): recommended type checked auto fixes

* chore(eslint): disable typed rules for __tests__

* chore(eslint): disable @typescript-eslint/unbound-method

We use unbound methods a *lot*.

* chore(eslint): a few manual fixes

* chore(eslint): enable typed rules for non-build files

* chore(eslint): disable more typed rules

* chore(eslint): some rule fixes

* chore(eslint): disable more typed rules

* chore(eslint): more manual lint fixes

* chore: remove todo

* chore(eslint): optimize overrides
  • Loading branch information
wjhsf authored Feb 14, 2024
1 parent d9588c7 commit 090aab8
Show file tree
Hide file tree
Showing 67 changed files with 214 additions and 140 deletions.
75 changes: 61 additions & 14 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,20 @@

"parser": "@typescript-eslint/parser",
"parserOptions": {
"sourceType": "module"
"sourceType": "module",
"project": true
},

"plugins": ["jest", "@lwc/lwc-internal", "@typescript-eslint", "import", "header"],
"extends": ["eslint:recommended", "plugin:@typescript-eslint/eslint-recommended"],
"extends": ["eslint:recommended", "plugin:@typescript-eslint/recommended-type-checked"],

"env": {
"es6": true
},

"reportUnusedDisableDirectives": true,
"rules": {
"no-unused-vars": "off",
"@typescript-eslint/no-unused-vars": ["error", { "argsIgnorePattern": "^_" }],

"block-scoped-var": "error",
"no-alert": "error",
"no-buffer-constructor": "error",
Expand All @@ -28,7 +28,6 @@
"no-iterator": "error",
"no-lone-blocks": "error",
"no-proto": "error",
"no-prototype-builtins": "error",
"no-new-require": "error",
"no-restricted-properties": [
"error",
Expand Down Expand Up @@ -139,21 +138,69 @@
" * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT",
" "
]
]
],
/*
* TODO: Address violations and re-enable these rules
*/
// All @ts-ignore should either be refactored or changed to @ts-expect-error <reason>
"@typescript-eslint/ban-ts-comment": "off",
// Enums are a pain to deal with...
"@typescript-eslint/no-unsafe-enum-comparison": "off",
// We might just want to leave this one disabled
"@typescript-eslint/unbound-method": "off",
// We use objects in template strings (e.g. `${vm}`) a lot - that's not that helpful for
// the user because it all just becomes "[object Object]"
"@typescript-eslint/no-base-to-string": "off",
"@typescript-eslint/restrict-template-expressions": "off",
// The following all derive from our liberal use of `any`
"@typescript-eslint/no-explicit-any": "off",
"@typescript-eslint/no-redundant-type-constituents": "off",
"@typescript-eslint/no-unsafe-member-access": "off",
"@typescript-eslint/no-unsafe-call": "off",
"@typescript-eslint/no-unsafe-assignment": "off",
"@typescript-eslint/no-unsafe-return": "off",
"@typescript-eslint/no-unsafe-argument": "off"
},

"overrides": [
{
"files": ["**/packages/lwc/**"],
"files": [
"**/__tests__/**",
"packages/@lwc/*/scripts/**",
// Just a weird edge case of a file...
"packages/@lwc/synthetic-shadow/index.js"
],
"parserOptions": {
"project": "./tsconfig.eslint.json"
}
},
{
// Not covered by any tsconfig, so typed rules won't work, but we don't need them anyway
"files": ["jest.config.js"],
"extends": ["plugin:@typescript-eslint/disable-type-checked"]
},
{
// Some tooling still uses require...
"files": [
"**/scripts/**",
"jest.config.js",
"packages/@lwc/integration-tests/src/components/**/*.spec.js"
],
"rules": {
"@typescript-eslint/no-var-requires": "off"
}
},
{
"files": ["packages/lwc/**"],
"rules": {
"no-restricted-imports": "off"
}
},
{
"files": [
"**/@lwc/engine-core/**",
"**/@lwc/engine-dom/**",
"**/@lwc/synthetic-shadow/**"
"packages/@lwc/engine-core/**",
"packages/@lwc/engine-dom/**",
"packages/@lwc/synthetic-shadow/**"
],
"rules": {
"@lwc/lwc-internal/no-global-node": "error",
Expand All @@ -162,7 +209,7 @@
}
},
{
"files": ["**/__tests__/**", "**/__mocks__/**", "**/@lwc/integration-karma/**"],
"files": ["**/__tests__/**", "**/__mocks__/**", "packages/@lwc/integration-karma/**"],

"env": {
"jest": true,
Expand All @@ -178,15 +225,15 @@
}
},
{
"files": ["**/@lwc/integration-tests/**"],
"files": ["packages/@lwc/integration-tests/**"],

"globals": {
"$": true,
"browser": true
}
},
{
"files": ["./*.js", "**/scripts/**", "**/jest.config.js"],
"files": ["./*.js", "**/scripts/**", "jest.config.js"],

"env": {
"node": true,
Expand All @@ -198,7 +245,7 @@
}
},
{
"files": ["**/perf-benchmarks/**"],
"files": ["packages/@lwc/perf-benchmarks/**"],

"globals": {
"after": true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ function validateSingleApiDecoratorOnSetterGetterPair(
state: LwcBabelPluginPass
) {
// keep track of visited class methods
const visitedMethods = new Set<String>();
const visitedMethods = new Set<string>();

decorators.forEach((decorator) => {
const { path, decoratedNodeType } = decorator;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ function validateImportedLwcDecoratorUsage(
// - an identifier @track : the decorator is the parent of the identifier
// - a call expression @wire("foo") : the decorator is the grand-parent of the identifier
const decorator = reference.parentPath!.isDecorator()
? reference.parentPath!
? reference.parentPath
: reference.parentPath!.parentPath!;

if (!decorator.isDecorator()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,11 +139,7 @@ function getGeneratedConfig(t: BabelTypes, wiredValue: WiredValue) {
const paramConfigValue = generateParameterConfigValue(memberExprPaths);

configProps.push(
t.objectProperty(
(param as types.ObjectProperty).key,
paramConfigValue.configValueExpression,
param.computed
)
t.objectProperty(param.key, paramConfigValue.configValueExpression, param.computed)
);

if (paramConfigValue.varDeclaration) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export default function (): Visitor<LwcBabelPluginPass> {
}

if (!state.dynamicImports.includes(dependency)) {
state.dynamicImports!.push(dependency);
state.dynamicImports.push(dependency);
}
}

Expand Down
84 changes: 41 additions & 43 deletions packages/@lwc/engine-core/src/framework/base-lightning-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,46 +143,45 @@ export interface LightningElementConstructor {
stylesheets: TemplateStylesheetFactories;
}

type HTMLElementTheGoodParts = Pick<Object, 'toString'> &
Pick<
HTMLElement,
| 'accessKey'
| 'addEventListener'
| 'attachInternals'
| 'children'
| 'childNodes'
| 'classList'
| 'dir'
| 'dispatchEvent'
| 'draggable'
| 'firstChild'
| 'firstElementChild'
| 'getAttribute'
| 'getAttributeNS'
| 'getBoundingClientRect'
| 'getElementsByClassName'
| 'getElementsByTagName'
| 'hasAttribute'
| 'hasAttributeNS'
| 'hidden'
| 'id'
| 'isConnected'
| 'lang'
| 'lastChild'
| 'lastElementChild'
| 'ownerDocument'
| 'querySelector'
| 'querySelectorAll'
| 'removeAttribute'
| 'removeAttributeNS'
| 'removeEventListener'
| 'setAttribute'
| 'setAttributeNS'
| 'spellcheck'
| 'tabIndex'
| 'tagName'
| 'title'
>;
type HTMLElementTheGoodParts = { toString: () => string } & Pick<
HTMLElement,
| 'accessKey'
| 'addEventListener'
| 'attachInternals'
| 'children'
| 'childNodes'
| 'classList'
| 'dir'
| 'dispatchEvent'
| 'draggable'
| 'firstChild'
| 'firstElementChild'
| 'getAttribute'
| 'getAttributeNS'
| 'getBoundingClientRect'
| 'getElementsByClassName'
| 'getElementsByTagName'
| 'hasAttribute'
| 'hasAttributeNS'
| 'hidden'
| 'id'
| 'isConnected'
| 'lang'
| 'lastChild'
| 'lastElementChild'
| 'ownerDocument'
| 'querySelector'
| 'querySelectorAll'
| 'removeAttribute'
| 'removeAttributeNS'
| 'removeEventListener'
| 'setAttribute'
| 'setAttributeNS'
| 'spellcheck'
| 'tabIndex'
| 'tagName'
| 'title'
>;

type RefNodes = { [name: string]: Element };

Expand Down Expand Up @@ -232,7 +231,6 @@ export const LightningElement: LightningElementConstructor = function (
);
}

const component = this;
setPrototypeOf(elm, bridge.prototype);

vm.component = this;
Expand All @@ -251,7 +249,7 @@ export const LightningElement: LightningElementConstructor = function (
markLockerLiveObject(this);

// Linking elm, shadow root and component with the VM.
associateVM(component, vm);
associateVM(this, vm);
associateVM(elm, vm);

if (vm.renderMode === RenderMode.Shadow) {
Expand Down Expand Up @@ -687,7 +685,7 @@ LightningElement.prototype = {
refsCache.set(refVNodes, refs);
}

return refs!;
return refs;
},

// For backwards compat, we allow component authors to set `refs` as an expando
Expand Down
2 changes: 1 addition & 1 deletion packages/@lwc/engine-core/src/framework/freeze-template.ts
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ function addLegacyStylesheetTokensShim(tmpl: Template) {
// If the value is null or some other exotic object, you would be broken anyway in the past
// because the engine would try to access hostAttribute/shadowAttribute, which would throw an error.
// However it may be undefined in newer versions of LWC, so we need to guard against that case.
this.stylesheetToken = isUndefined(value) ? undefined : (value as any).shadowAttribute;
this.stylesheetToken = isUndefined(value) ? undefined : value.shadowAttribute;
},
});
}
Expand Down
4 changes: 2 additions & 2 deletions packages/@lwc/engine-core/src/framework/hydration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ function hydrateCustomElement(
hydrated: true,
});

vnode.elm = elm as Element;
vnode.elm = elm;
vnode.vm = vm;

allocateChildren(vnode, vm);
Expand All @@ -346,7 +346,7 @@ function hydrateCustomElement(
const { getFirstChild } = renderer;
// VM is not rendering in Light DOM, we can proceed and hydrate the slotted content.
// Note: for Light DOM, this is handled while hydrating the VM
hydrateChildren(getFirstChild(elm), vnode.children, elm as Element, vm, false);
hydrateChildren(getFirstChild(elm), vnode.children, elm, vm, false);
}

hydrateVM(vm);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ function getMapFromClassName(className: string | undefined): Record<string, bool
return EmptyObject;
}
// computed class names must be string
// This will throw if className is a symbol or null-prototype object
// eslint-disable-next-line @typescript-eslint/restrict-plus-operands
className = isString(className) ? className : className + '';

let map = classNameToClassMap[className];
Expand Down
5 changes: 3 additions & 2 deletions packages/@lwc/engine-core/src/framework/rendering.ts
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ function mountStatic(

if (isSyntheticShadowDefined) {
if (shadowMode === ShadowMode.Synthetic || renderMode === RenderMode.Light) {
(elm as any)[KEY__SHADOW_STATIC] = true;
elm[KEY__SHADOW_STATIC] = true;
}
}

Expand Down Expand Up @@ -535,7 +535,7 @@ function updateTextContent(vnode: VText | VComment, renderer: RendererAPI) {
if (process.env.NODE_ENV !== 'production') {
unlockDomMutation();
}
setText(elm, text!);
setText(elm, text);
if (process.env.NODE_ENV !== 'production') {
lockDomMutation();
}
Expand Down Expand Up @@ -815,6 +815,7 @@ function allocateInSlot(vm: VM, children: VNodes, owner: VM) {
// but elm.setAttribute('slot', Symbol(1)) is an error.
// the following line also throws same error for symbols
// Similar for Object.create(null)
// eslint-disable-next-line @typescript-eslint/restrict-plus-operands
const normalizedSlotName = '' + slotName;

const vnodes: VNodes = (cmpSlotsMapping[normalizedSlotName] =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ function initGlobalStylesheet() {
return promise;
}
);
// eslint-disable-next-line @typescript-eslint/no-floating-promises
Promise.all(promises).then((stylesheetTexts) => {
// When replaceSync() is called, the entire contents of the constructable stylesheet are replaced
// with the copied+concatenated styles. This means that any shadow root's adoptedStyleSheets that
Expand Down
1 change: 0 additions & 1 deletion packages/@lwc/engine-core/src/framework/template.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ function validateSlots(vm: VM) {
const { cmpSlots } = vm;

for (const slotName in cmpSlots.slotAssignments) {
// eslint-disable-next-line @lwc/lwc-internal/no-production-assert
assert.isTrue(
isArray(cmpSlots.slotAssignments[slotName]),
`Slots can only be set to an array, instead received ${toString(
Expand Down
1 change: 1 addition & 0 deletions packages/@lwc/engine-core/src/framework/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export function addCallbackToNextTick(callback: Callback) {
}
}
if (nextTickCallbackQueue.length === 0) {
// eslint-disable-next-line @typescript-eslint/no-floating-promises
Promise.resolve().then(flushCallbackQueue);
}
ArrayPush.call(nextTickCallbackQueue, callback);
Expand Down
4 changes: 2 additions & 2 deletions packages/@lwc/engine-core/src/framework/vm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ function validateComponentStylesheets(vm: VM, stylesheets: TemplateStylesheetFac
const validate = (arrayOrStylesheet: TemplateStylesheetFactories | StylesheetFactory) => {
if (isArray(arrayOrStylesheet)) {
for (let i = 0; i < arrayOrStylesheet.length; i++) {
validate((arrayOrStylesheet as TemplateStylesheetFactories)[i]);
validate(arrayOrStylesheet[i]);
}
} else if (!isFunction(arrayOrStylesheet)) {
// function assumed to be a stylesheet factory
Expand Down Expand Up @@ -673,7 +673,7 @@ function flushRehydrationQueue() {

// re-throwing the original error will break the current tick, but since the next tick is
// already scheduled, it should continue patching the rest.
throw error; // eslint-disable-line no-unsafe-finally
throw error;
}
}

Expand Down
Loading

0 comments on commit 090aab8

Please sign in to comment.