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

Feature request: export another endpoint that doesn't depend on function-bind and possibly other polyfills #476

Closed
jedwards1211 opened this issue Jun 27, 2023 · 21 comments

Comments

@jedwards1211
Copy link

In my work I'm maintaining a package that prioritizes supporting modern environments and bundlers out of the box over supporting ancient environments. When bundling with Next.js I'm getting the error about dynamic code evaluation in function-bind and I see that qs uses a heady compatibility layer to support old environments.

The old environment support is holding back the happy path for people using modern bundlers; for us it hurts more than it helps. So, would it be too much trouble for qs to export an alternative entrypoint that doesn't rely on this compatibility layer?

Our other options are not very good:

  • Downgrade to an old version of qs
  • Try to find an alternative to qs with the same semantics
  • Tell users how to configure their bundlers to ignore these errors
@ljharb
Copy link
Owner

ljharb commented Jun 27, 2023

I really don't agree that it hurts more than it helps; "more bytes than needed" is barely an inconvenience.

The proper approach is for packages - which can make no assumptions about their target environments - to be maximally backwards-compatible and safe, and for apps - where target environments are known and bundlers/build tools are in use - to do whatever patches are needed.

For example, if you only support environments where Function.prototype.bind exists, you can tell your bundler to replace require('function-bind') with Function.bind, and remove the package entirely. You can similarly replace has with Object.hasOwn if that's always available.

@jedwards1211
Copy link
Author

I am saying this hurts us more than it helps. I respect your goals to provide maximum backwards compatibility but our goals are different and I would like to see if we can come to some kind of compromise that doesn't involve telling our users to futz with bundler configuration.

@jedwards1211
Copy link
Author

jedwards1211 commented Jun 27, 2023

Maintaining support for old versions of phantomjs, Internet Explorer < 9, and node < 0.6 doesn't make any kind of business sense for us when it complicates the happy path for our users, none of whom are using our packages in those environments.

@ljharb
Copy link
Owner

ljharb commented Jun 27, 2023

Understood.

I'm not sure it's a viable compromise to add additional weight - and a redundant implementation, forcing all future changes to be made in two places - versus you making a bundler config/plugin that has all the tweaks and users can easily use?

@ljharb
Copy link
Owner

ljharb commented Jun 27, 2023

Especially considering that even if I did what you asked for qs, I maintain hundreds of packages that follow these same patterns - using a bundler is a holistic approach that can work for every current and future instance of this pattern, instead of just trying to fix it one npm package at a time.

@jedwards1211
Copy link
Author

jedwards1211 commented Jun 27, 2023

what about using an export map to override a module that exports side-channel with a non-polyfilled implementation in more modern environments/bundlers that read the export map? For example

// stringify.js
const getSideChannel = require('qs/side-channel')
{
  "name": "qs",
  "exports": {
    "./side-channel": {
      "legacy": "./side-channel.js",
      "default": "./side-channel-nopolyfill.js"
    }
  }
}

The minority of users who would need to support really old environments could set the legacy exports condition in their bundler if necessary

@jedwards1211
Copy link
Author

jedwards1211 commented Jun 27, 2023

I think when it comes to really old environments it makes more sense for the compatibility polyfills to be opt-in. That seems to be the philosophy of most other packages I've dealt with...in general the Node ecosystem drops support for old environments too quickly IMO, so I like that you care about maintaining more backwards compatibility, but supporting old versions of phantomjs, Internet Explorer < 9, and node < 0.6 without manual configuration in this day and age seems extreme in the opposite direction. I think most devs would expect to need to manually configure polyfills for many packages to work in such old environments now.

@jedwards1211
Copy link
Author

I'm not always opposed to configuration, but this is the only package that's currently an obstacle to a zero-config solution for us, and if all packages in the ecosystem provided backwards compatibility in this way, I can imagine a scenario where I'd have to configure dozens of overrides in my bundler just to get it to build.

@jedwards1211
Copy link
Author

I guess another question is - is there truly no way for side-channel to work without doing a .bind for some reason?

@ljharb
Copy link
Owner

ljharb commented Jun 27, 2023

To be clear, call-binding has nothing to do with old implementations - it's about robustness. The only part that's for old impls is the function-bind package - but the actual binding is critical.

could set the legacy exports condition in their bundler if necessary

That doesn't work for node, where bundlers are usually not used.

I think when it comes to really old environments it makes more sense for the compatibility polyfills to be opt-in.

Age doesn't factor in for me.

For side-channel, if you only support envs with WeakMap, some amount of the code could be removed, but call-bind depends on function-bind as mentioned, and call-bind itself is necessary. object-inspect, similarly, uses call-binding all over the place to be robust against builtin modification.

@jedwards1211
Copy link
Author

jedwards1211 commented May 4, 2024

Jordan, does a design being better for 1% of users and worse for 99% of users factor in for you? Or do you prefer to willfully ignore the downsides obsessive backward compatibility has for most library users?

You've done a lot of good for the community in your work on standards, but your stubbornness in this case makes things more difficult for most developers.

@ljharb
Copy link
Owner

ljharb commented May 4, 2024

The downsides aren’t relevant for 99% of users though - you’ve explained your use case, but i don’t see any benefit in zero-config (you miss out on gobs of bundle size optimizations that way, no matter what individual packages do), and the costs and benefits aren’t equal - you’re asking for that 1% to be totally fucked, versus the 99% being mildly inconvenienced.

@jedwards1211
Copy link
Author

jedwards1211 commented May 4, 2024

This discussion has ranged but I started out asking for an opt-in endpoint. This wouldn't fuck over the 1% of users because they wouldn't opt into it. The other 99% could opt in and be less inconvenienced.

Is there any way you can see for us to opt in to something that results in smaller bundle sizes for us? Any way at all besides patching?

@jedwards1211
Copy link
Author

jedwards1211 commented May 4, 2024

The proper approach is for packages - which can make no assumptions about their target environments - to be maximally backwards-compatible and safe, and for apps - where target environments are known and bundlers/build tools are in use - to do whatever patches are needed.

How do you really see this working if every package out there took this approach? In a production project with hundreds of dependencies, this would mean patching hundreds of packages that each provide compatibility shims in different ways. Is that really what you think the ideal production app development workflow looks like?

@ljharb
Copy link
Owner

ljharb commented May 5, 2024

In practice, that would mean that you'd use a bundle config that replaced multiple packages that do the same thing, all with a single implementation. You can do that right now (or at any time in the past 10 years) and you'd get benefits - there's lots of packages that are 1:1 duplicates of each other that you can consolidate with overrides and/or bundler config.

So yes, this is exactly what the ideal production app development workflow looks like, even if i changed all 500+ of my packages to drop node < 18.

@jedwards1211
Copy link
Author

jedwards1211 commented May 5, 2024

If you have an example of where you've actually done that for a significant number of dependencies in a large project and it felt like, yes, this is a wonderful development workflow, then I'm happy to see what it looks like. Because it seems to me it would take a lot of researching what shimming each individual package and its transitive dependencies use, and coming up with an override for each one.

And I mean if everything in the ecosystem depended on shims for ES3 compatibility then ideally there would just be a core few common dependencies to override or many would be fully interchangeable, but I wouldn't expect it to work out that way in practice?

Whereas on the other hand a lot of packages could abide by some kind of convention like if we manually set an import condition like "ES5" or "ES2020" in our bundler or environment, then many packages would automatically provide an entrypoint that doesn't depend on shims for earlier environments. I imagine something like that would be a lot more convenient for everyone to work with?

@ljharb
Copy link
Owner

ljharb commented May 6, 2024

Airbnb did that while I worked there; that was 5 years ago but I assume it's still similar.

Yes, it does require a lot of research, and the nature of package reuse means that the effort required dramatically drops over time.

I also agree that it would be beneficial for an OSS tool to exist that collects this information.

@jedwards1211
Copy link
Author

I also agree that it would be beneficial for an OSS tool to exist that collects this information.

Finally, we are getting somewhere.

Should a single centralized OSS tool collect this information on as many packages as possible? Or one centralized for webpack, one for other bundlers, etc?

...or, would it make more sense for that information to be distributed among packages, which would volunteer information about themselves according to some kind of convention?

Again, one such convention could be standardized export conditions in export maps.

@ljharb
Copy link
Owner

ljharb commented May 6, 2024

Either way, arbitrarily picking a time-based snapshot of a living spec wouldn't be useful at all :-)

@jedwards1211
Copy link
Author

So something more like @babel/preset-env where packages could declare what shims to leave out based upon the capabilities of the requested target environments?

@ljharb
Copy link
Owner

ljharb commented May 6, 2024

Yea, that’s be how such a tool would need to work.

Happy to keep discussing, but until such a convention and tool exists, it wouldn’t make sense for any package to follow it, and I’ve already explained why having two sources of truth inside a package is dangerous and unmaintainable, so I’ll close this for now.

@ljharb ljharb closed this as completed May 6, 2024
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

2 participants