perf: do not bind plugin hook context functions

[patak-dev]

Description

This PR reverts

• fix(dev): bind plugin context functions #14569

Doing the 14 binds on each created plugin hook Context represents ~1.2% of the total work of the server using vite-dev-server-perf. Given that this feature isn't really needed, and the issue that triggered it ended up in the plugin being modified to work without binded functions I don't think we can justify the perf penalty.

I think we're still in time to revert this in Vite 5.1 given that it was added in Vite 5.0 but we didn't documented it (and it is not documented in rollup either). cc @bluwy @sapphi-red

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[bluwy]

IIUC Rollup intends that its plugin context can be passed around without users manually binding it. There's also a recent PR that remove the last this restriction within Rollup (rollup/rollup#5270). So removing our binds here means we're not compatible with Rollup.

If we want to fix the performance of this. I think we might need to rewrite the classes as closures. That might also make TransformContext a little tricky though.

[patak-dev]

Moving to closures is probably going to be slower than what we currently have using bind functions as all the functions need to be recreated on every context creation. See this perf.link. As I was adding more complexity in the function, the closures got more and more slower.

Given that Vite didn't support this.resolve as binded function until Vite 5.0 and that this is not documented in Rollup, I think nobody expects it to be binded. I would understand this should be required if resolve was passed as a function in an object param to the hooks, but going through this puts the expectations that users must binded manually when needed to me. Even if I was advocating to follow rollup during the discussions in #14569, I still think we made the wrong call and it we don't have a good use case for this perf penalty.

[bluwy]

Funny how we switched sides here 😄 Thanks for testing the perf for closures, interesting to see classes winning here. Maybe we should loop in @lukastaegert about this. I'd feel more comfortable if we have Rollup acknowledge the intentional divergent here, since we'd need to get one of its upstream PR resolved still: rollup/plugins#1603

[lukastaegert]

That is interesting indeed.
Rollup (mostly) avoids closure re-creation by caching plugin contexts, but I agree that it allows for some more efficient scenarios if we drop this requirement. Maybe something for Rollup 5. In the meantime, I think it is fine if Rollup and Vite diverge in this point as long as Vite is "stricter" than Rollup, i.e. users need to take more care to explicitly bind context functions.