chore(client)!: remove never implemented hot.decline

[ArnaudBarre]

Re: #11004

[bluwy]

Re #11004 (comment)

@bluwy maybe you could check why Astro is using it? It isn't doing anything on Vite's side. Maybe they believe it is working and it is hiding bugs (probably it was implemented in Snowpack).

Yeah it's likely a remnant from Snowpack HMR days, but I don't think we should break it. It still has an intent and is future-proof, e.g. if one day Astro decides to use a different framework that implements .decline().

I think we should document it and type it still, but maybe put a note that it's currently a no-op.

[patak-dev]

Agree that we could leave it if there was a good reason for it. But I wonder why Vite doesn't need to implement it.

But does it makes sense that Astro is using it? It looks hot.decline was added by @natemoo-re only a few months ago withastro/astro#3932

[ArnaudBarre]

I didn't see any clear intent for it.
Either:

• you know ahead of time HMR is not possible and just don't append anything to the file
• you know at runtime you need to invalidate and call hot.invalidate
• you need to undo some side effects and use hot.dispose
• you know the only possible way to recover is to reload the page and call location.reload

The last point was probably the intended use, but adding an entry to an API that is just an alias for a well know supported platform feature feels wrong.

If another need appear in the feature, I think it's better to discuss it and find a new name in the API for it

[patak-dev]

@FredKSchott maybe you could shed some light on why hot.decline was added to the API? Maybe it is there for historic reasons. It could have been added first, then invalidate was added later and made the former a bit redundant.

[natemoo-re]

But does it makes sense that Astro is using it? It looks hot.decline was added by @natemoo-re only a few months ago withastro/astro#3932

Probably not! I was just throwing things at the wall trying to fix some HMR stuff. If it's a noop this can be removed.

[FredKSchott]

Yea, there's a lot of history here from the early Snowpack and Vite days. You can see the early repo where Evan and I were exploring implementing the same API: https://github.com/FredKSchott/esm-hmr

You can see this API spec'd out there: https://github.com/FredKSchott/esm-hmr#decline

It's been long enough that I don't remember the history of why this was needed, but I have no need to keep it around at this point if its a no-op. I'm overdue to add a note to that repo README that the idea of a shared "spec" is abandoned, and to redirect people to Vite's HMR docs as the most likely up-to-date docs.

[bluwy]

I still feel like we shouldn't remove it just because we don't implement it though. Webpack also implements decline and perhaps we should implement the full reload if it's not working now.

If another need appear in the feature, I think it's better to discuss it and find a new name in the API for it

I don't think we should create a new API name when there's prior art.

---

I'm maybe in the outlier here 😬 But I think we should avoid breaking things unless there's a compelling reason besides cleaning things up.

[patak-dev]

I think the fact that webpack implements it is a good point in favor of keeping hot.decline. FWIW, I thought the idea of removing it was good not as a clean-up but to avoid confusing users. Because currently, as it happened to Nate, they may think that the API is actually having an effect. We could leave it with the justification of backward compat and document that this API is a noop in Vite, and point to what should be used instead to achieve the same.

[ArnaudBarre]

For me it should at least be remove from types, because people will read their autocomplete before reading the docs and it can takes sometimes before seeing that it does nothing and go reading the doc

[bluwy]

Removing from the types sounds like a nice middle ground for me too. And also updating the docs regarding it being a no-op and could be implemented in the future, plus directing what to use instead.

[ArnaudBarre]

@bluwy @patak-dev Are you ok with this version?

[bluwy]

Looks good to me 👍