chore: replace standard and snazzy with neostandard by Uzlopak · Pull Request #3485

chore: replace standard and snazzy with neostandard by Uzlopak · Pull Request #3485 · nodejs/undici

Migrate from standard and snazzyy to neostandard by @voxpelli . Also starting to lint typescript files.

To make it easier to review:

First commit is just setting up eslint with neostandard instead of standard and snazzy, + configure for our project
Second commit is with automatic fixing via --fix flag
Third commit is manually fixing linting errors

Uzlopak changed the title ~~Use neostandard~~ chore: replace standard and snazzy with neostandard

Aug 18, 2024

Choose a reason for hiding this comment

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

LGTM 🚀

ronag left a comment •

Loading

Choose a reason for hiding this comment

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

Why? If we are going this way I'd rather go with eslint + prettier.

		}),
		{
		rules: {
		'@stylistic/comma-dangle': ['error', {

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

If you want, you can update to 0.11.3 now and this override will no longer be needed (though having it will enforce no dangling commas if that's desireable)

		exports: 'never',
		functions: 'never'
		}],
		'@typescript-eslint/no-redeclare': 'off',

Choose a reason for hiding this comment

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

Was this a particular issue with TS-files? Maybe it's something we should tweak in neostandard itself?

Choose a reason for hiding this comment

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

this basically happens if you have something like

export interface CacheStorage {
  match (request: RequestInfo, options?: MultiCacheQueryOptions): Promise<Response | undefined>,
  has (cacheName: string): Promise<boolean>,
  open (cacheName: string): Promise<Cache>,
  delete (cacheName: string): Promise<boolean>,
  keys (): Promise<string[]>
}

declare const CacheStorage: {
  prototype: CacheStorage
  new(): CacheStorage
}

Because the interface CacheStorage exists declaring the const CacheStorage results in a no-redeclare linting error

Choose a reason for hiding this comment

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

lgtm

Why? If we are going this way I'd rather go with eslint + prettier.

prettier is incompatible with standard so it would be impossible to do backports. neostandard is just an eslint config and backward compatible with standard.

I worked now together with @voxpelli regarding this PR, because the linting was quite slow (~10s). Disabling jsx to avoid running react related linting saved 2 s. Also activated eslint cache makes it run significantly faster.

So the first run is slow but consecutive runs will have the same perf as before.

Well. I think the PR is now mature enough for the merge.

So if @ronag 's is ok with it, then we could merge it :).

Choose a reason for hiding this comment

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

lgtm

The backport to v6.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-v6.x v6.x
# Navigate to the new working tree
cd .worktrees/backport-v6.x
# Create a new branch
git switch --create backport-3485-to-v6.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 26004bfccaead3d6ec143c03a09dc036cea2c2ae
# Push it to GitHub
git push --set-upstream origin backport-3485-to-v6.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-v6.x

Then, create a pull request where the base branch is v6.x and the compare/head branch is backport-3485-to-v6.x.

This was referenced

Dec 16, 2024

Choose a reason for hiding this comment

ronag left a comment • Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ronag left a comment •

Loading