Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "doc: promote fetch/webstreams from experimental to stable" #49867

Closed
wants to merge 1 commit into from

Conversation

KhafraDev
Copy link
Member

@KhafraDev KhafraDev commented Sep 25, 2023

This reverts commit 740ca5423aabc06af48f6dacfb17335bbaa25c0c1.

At minimum, this needs more discussion.

Refs: nodejs/undici#1737 (comment) (discussion starts here)

This reverts commit 740ca5423aabc06af48f6dacfb17335bbaa25c0c1.

At minimum, this needs more discussion.

Refs: nodejs/undici#1737 (comment)
@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Sep 25, 2023
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

I’m -1, let’s wait for the discussion in Undici to complete.

@KhafraDev
Copy link
Member Author

There doesn't seem to be any discussion going on, but I would like to continue it. Not sure what else there is to discuss because my position hasn't changed but I can reiterate what I've said, I guess, if anyone doesn't want to read a hundred posts. lol

@mcollina mcollina added the tsc-agenda Issues and PRs to discuss during the meetings of the TSC. label Oct 15, 2023
@GeoffreyBooth
Copy link
Member

Not sure what else there is to discuss

What are you proposing, exactly? I think there isn't support for keeping things experimental forever. Is there some other status that would better represent our stability guarantee other than calling this stable?

@KhafraDev
Copy link
Member Author

@GeoffreyBooth
Copy link
Member

nodejs/undici#1737 (comment)

So can you open a PR to propose an implementation of that?

@KhafraDev
Copy link
Member Author

I haven't gotten a response as whether it's a good idea, or at least an acceptable alternative. There were people who were against adding a new stability index earlier in the discussion but they seem to have stopped responding (I don't blame them, very few people have reason to care about this).

@GeoffreyBooth
Copy link
Member

Well, opening a PR is one way to force the issue. If they don't care enough to respond they probably don't care enough to object.

Personally I don't think we can leave things as experimental forever. Our users want a stability guarantee. So if fetch needs breaking changes more frequently than in each major, please propose a new stability level to describe what exactly that means. I think drafting a PR is the best way because then we know exactly what you're proposing and we can debate the PR text.

@mcollina
Copy link
Member

I'm -1 because we can't leave things experimental forever. If we need to loosen the definition of "stable" to compensate for semver-major spec changes, let's do it.

@KhafraDev
Copy link
Member Author

Should the definition of stable be loosened, or should a new stability index be added? Loosening it might cause more breaking changes, which might be wanted (going on some recent twitter discussions)?

@GeoffreyBooth I can't promise that I will make a PR, I'm very busy currently unfortunately. From the undici discussion, no one wants to keep it experimental forever, it's just that experimental is a better label, currently, than anything else.

@ronag
Copy link
Member

ronag commented Oct 26, 2023

The way I see it:

stable means that we don't ship breaking changes. If we implement a breaking fetch change then that should be a semver major for undici and we only bring in semver major changes into node when doing semver major in node.

If someone needs the latest, greatest and most web compatible fetch implementation they can use the undici package directly.

@KhafraDev
Copy link
Member Author

but then we have potential issues where we cannot land undici PRs in node core for months to a year still, and LTS releases are screwed too. Node core also now exposes WebSocket from undici which means by association we can't land fixes for it either, although it's experimental.

@ronag
Copy link
Member

ronag commented Oct 26, 2023

I don't see a problem. Use the npm package if you need the latest.

I don't think this is an issue in practice for web socket. If and when it becomes an issue I'm happy to reconsider.

@GeoffreyBooth
Copy link
Member

I don’t see a problem. Use the npm package if you need the latest.

This might be a good compromise. We could at least mention this in the docs, with examples, like “here’s how to override Node’s global with the version from your locally installed Undici”.

@KhafraDev
Copy link
Member Author

If and when it becomes an issue I'm happy to reconsider.

this is already an issue, there's already a breaking change to fetch that can't land in undici because fetch is stable 😐

@KhafraDev
Copy link
Member Author

It's also not a great compromise - why make fetch a global if there's a serious need to override it (whether it's security, being up to date, bug fixes, etc.)? I mentioned this in the undici issue.

@ronag
Copy link
Member

ronag commented Oct 29, 2023

If and when it becomes an issue I'm happy to reconsider.

this is already an issue, there's already a breaking change to fetch that can't land in undici because fetch is stable 😐

Land it and we make a semver major release of undici? Also might make sense to move all the web spec stuff into separate repo.

@ronag
Copy link
Member

ronag commented Oct 29, 2023

It's also not a great compromise - why make fetch a global if there's a serious need to override it (whether it's security, being up to date, bug fixes, etc.)? I mentioned this in the undici issue.

IMHO. The only need for global is for isomorphic usage. And that people expect it.

@ronag
Copy link
Member

ronag commented Oct 29, 2023

Not saying this is an optimal situation. But I don't see a better alternative.

@ronag ronag removed the tsc-agenda Issues and PRs to discuss during the meetings of the TSC. label Nov 8, 2023
@KhafraDev KhafraDev closed this May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants