Skip to content

chore: introducing @e18e/eslint-plugin - #23357

Open
btea wants to merge 6 commits into
vitejs:mainfrom
btea:chore/enable-e18e-eslint
Open

chore: introducing @e18e/eslint-plugin#23357
btea wants to merge 6 commits into
vitejs:mainfrom
btea:chore/enable-e18e-eslint

Conversation

@btea

@btea btea commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Introducing the e18e/eslint-plugin, discarding some old syntax, and replacing it with new APIs and syntax supported by Node 20+. https://github.com/e18e/eslint-plugin#rules

@btea
btea marked this pull request as draft August 25, 2026 02:46
const moduleId = meta.id

const importee = callstack[callstack.length - 1]
const importee = callstack.at(-1)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

#12539 (review) Follow the previous expectations

@btea
btea marked this pull request as ready for review August 25, 2026 04:36
@bluwy

bluwy commented Aug 31, 2026

Copy link
Copy Markdown
Member

I think we'd also need to double check the performance but I suppose most is fine. Except maybe .concat could be better sometimes.

It'll also be a bit tricky if we integrate this and have to migrate to oxlint later, but not a big reason to hold this PR off if all looks fine.

@btea

btea commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

I think we'd also need to double check the performance but I suppose most is fine. Except maybe .concat could be better sometimes.

I will add an API performance comparison example later. yes,concat cannot be directly replaced by the spread operator; my previous attempt to replace it directly caused some logical errors. I will further verify this later or simply remove this rule.

It'll also be a bit tricky if we integrate this and have to migrate to oxlint later, but not a big reason to hold this PR off if all looks fine.

Perhaps we can consider this together when we migrate to oxlint?

@bluwy

bluwy commented Sep 1, 2026

Copy link
Copy Markdown
Member

I think some quick performance tests is fine and I'll trust what you discover 😄 I don't think we need to wait for oxlint, but I'll leave to @sapphi-red if he wants to hold it off or something.

If the changes here doesn't cause big perf issues, I think we can merge it now. For me particularly I'm interested in the perf of Array.from and .concat, but it also doesn't feel necessary to move away from them to spread operators.

Comment thread packages/vite/src/node/ssr/fetchModule.ts
Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com>
@btea

btea commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

I did a simple test in a Node.js environment, and the performance of the spread operator and Array.from is almost the same, which is negligible.

The differences between concat and the spread operator are significant in different situations. When merging two arrays, concat is about three times faster overall. When concatenating a single element, the spread operator is two to three times faster.

Therefore, Array.from should not need to be replaced, and whether to replace concat depends on the situation. However, this is a simple local test, and it may be inaccurate.

concat vs spread(combined array) concat vs spread(single item)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants