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

should this library maintain support for isomorphism? #128

Open
ctcpip opened this issue Oct 22, 2024 · 3 comments
Open

should this library maintain support for isomorphism? #128

ctcpip opened this issue Oct 22, 2024 · 3 comments

Comments

@ctcpip
Copy link
Member

ctcpip commented Oct 22, 2024

In recent discussion, it was mentioned that this library is used by some downstream consumers in the browser, using browserify or similar mechanisms. (There is also some historical discussion on browser support.)

We cannot support browser environments passively -- if this library is expected to work in the browser, then we need to take steps to ensure that it runs in the browser and will continue to run in the browser. This is critical for back-compat but also for forward-compat. This should minimally involve adding some level of CI smoke testing in browsers.

Therefore, this issue aims to answer the question: should this library maintain support for isomorphism?

@wesleytodd
Copy link
Member

wesleytodd commented Oct 22, 2024

As the person making that case (in 2015 😬) I can for sure say that the ecosystem has solutions for this type of thing now which it did not have back then. The use case was never popular by any means, but I don't like saying we should explicitly not support it when it for sure works.

Frankly I think the fact that React's ecosystem chose to put routing IN the UI component tree was a design mistake. That said, who am I to argue with something so popular and the other frameworks I have not really worked with beyond toy examples to say much about them. Unless we can work out a really clear recommendation for folks serving UI's from express apps that rules out running any express code in the browser, I think this approach is "technically good".

Now, that would mean we need to setup testing, and that was the real blocker in the past. It is possible (I hope obviously), but no one was there to do the work. And unless we have someone to do that I would hesitate to commit to it. So I would lean toward asking if anyone was interested in helping get browser testing working, and then if we get someone in a reasonable time we say we support it and if not we officially drop support. Unfortunately because of #125, if we went that way, would need to be backed out until we make a formal decision (right? or is there another good option I am not thinking of?).

@blakeembrey
Copy link
Member

For this particular package, I don't think we should attempt to make it run in a browser. Any attempt at this point is probably mistaken, and with the latest major release I think it's fair we take that as the opportunity to flag to anyone who breaks that it only runs in node Vx and nowhere else. It seems fair to re-evaluate if there's strong feedback otherwise, but up to the maintainer, if it were me I'd note this is specifically "node http" supported.

@ctcpip
Copy link
Member Author

ctcpip commented Oct 24, 2024

I mentioned this in the meeting yesterday, but I did take a stab at running it and tests in the browser, and ran into a couple walls.

The status quo today is either:

  • it's not supported at all in the browser
  • it's supported in the browser in an implicit, hand-wavy, best effort basis

I am sympathetic to the latter, but as discussed in the meeting yesterday, since v2.0.0 dropped support for node <18, #125 does not need to be backed out because no major rev is needed here.

By saying we support minimum Node 18, for implicit browser support, that means we support minimum V8 10.1, therefore by extension, minimum supported browser versions are approximately:

  • Chrome 103
  • Firefox 103
  • Safari 15
  • Edge 103

All of these are WELL past when setPrototypeOf was available (2014 in chrome 34, 2015 in node 0.12)

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

No branches or pull requests

3 participants