-
Notifications
You must be signed in to change notification settings - Fork 6
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
Advice for library authors who use polyfills? #6
Comments
Hey Nolan, thanks for this really thoughtful feedback. We'll take this to the next face to face meeting but just one clarification: what instructions would be needed for module bundlers? My assumption is that producing two versions of your library (one with polyfills, one without) is not difficult, and developers wishing to use the 'bare' version would load the necessary polyfills globally, so would not need to recompile your lib. Did I miss something? |
I haven't really thought about as much from a library point of view @nolanlawson but you make a lot of good points. Oh are a lot of a library authors shipping polyfills? I guess that makes sense but to me it seemed like the general recommendation was that applications should be providing the polyfills (but of course like you said its least resistance for users then since then don't need to know that).
The instance where you don't include polyfills seems best to me but then the question is how do you signal to the user that it's required (readme, easy of finding one and including it, etc) From the app side, my purpose with |
I tend to be of the view that library authors should include any polyfills they're using (albeit in an encapsulated, non-polluting way per this repo's recommendations) that are required to work in the environments that the library supports, just like libraries should be distributed in a bundled and transpiled form. If I have to jump through hoops like rigging up an opinionated build step just to use your library, then your library is broken. Having an app that works is the priority, deduping a few kb of shared polyfills is low-hanging optimisation fruit for those with the necessary expertise. (This is consistent with the view that libraries in many cases shouldn't include polyfills by the way — it's totally fine for a library to say 'we only support environments that define I think the real difficulty is in communicating to consumers how to choose between two different versions of a library, since you basically have to provide examples for all the different tooling. And it gets exponentially harder for packages that have multiple modules rather than exposing a single entry point. I'm not sure there's a universal solution! But again, as long as it works by default, you're meeting the needs of your userbase while pushing the burden of extra configuration and optimisation onto the expert users who can bear it. |
@Rich-Harris that sounds good too! |
@hzoo transform-runtime actually results in even more bloat for the user, should the user already provide their own polyfills. Say, if the final user already loads babel-polyfill (includes core-js in shim mode) and a random dependency was built with transform-runtime, you now get a second copy of core-js bundled. |
Thanks for the feedback, everyone! To answer your questions, I'd mostly like to expand on what @Rich-Harris said because I think he nailed it. The problem is not so much technical (Babel/Rollup/Webpack already offer many solutions), but more a problem of documentation/standardization/ecosystem. E.g.:
Result: pandemonium, nobody knows how to configure their app to handle everybody's format.
Except when every library has a different list. :) E.g. one requires I do tend to agree that the default should be "polyfill all the things" (better from UX standpoint), and the alternative should be the "no polyfills" version for those who want to squeeze out more KBs. Question is if there's a way we can standardize around that, or at least promote good patterns for the ecosystem.
Another reason it'd be nice to crack this nut. 😃 One set of polyfills for the entire app codebase is ideal, but this requires coordination between app authors and library authors. |
I believe library authors should definitely ship polyfills. You specify (or should anyway) what versions of node/npm you support via package.json engines field and ultimately the lib should ship with everything needed in order to work in that version range. While it would lessen the bloat if you didn't include polyfills it would put a serious burden on you and other library authors that go that path. You would have to define these polyfills as peerDependencies because that is exactly what they would be. peerDependencies are rarely a good idea.. So I think library authors should definitely ship polyfills and then application authors now have the burden of creating the smallest bundles possible. Tree shaking and other methods could be used to help reduce the bloat from an application. |
Right that's a good point that we need both options (simple default, and ability to improve): users can have a hard time with Babel when dealing with polyfills or generators (via regenerator). We don't auto include them by default (or anything really atm) so it causes some pain. They will ask why "doesn't array.prototype.includes work (or get transpiled)". So we can point them to babel-polyfill (the whole thing) or transform-runtime, or the new useBuiltIn option for preset-env but there's a lot of work involved. It's also the use case of making a prod app or just testing out the library. |
But yeah if we had a tool to remove the libraries bundled polyfills automatically (sounds crazy) that would be pretty amazing too? And maybe in the future https://github.com/babel/babili or bundler like webpack could be a part of it |
Throughout the development of a library, its developer will make the decision to support some browsers or not. Should I include a Promise polyfill? How about a querySelectorAll polyfill? You, as a library user, would you prefer a 1KB module or a 10K module that's compatible with some old browser you don't care about? In short: it's up to the developer to include them ponyfill-style or not at all. They definitely should not clutter the global space. |
Maybe not standardisation in the strong sense, but I certainly think we can reach a community consensus. It might be a slightly painful process, and the bikeshed may get a few layers of paint before we find a colour we all like, but it's doable (consider things like My vote would be for a pattern based on filenames, for the following reasons:
I don't think this is just about polyfills, either. I'd be interested to see libraries shipping untranspiled, unbundled source code (in addition to bundled, transpiled and polyfilled distributables) to give power users the option to customise their builds.
This is where pragmatism > ideology. In that situation I'd choose the first. Realistically though, the choice is probably more often between, say, a 20kb unpolyfilled lib versus a 25kb polyfilled one, in which case I would probably choose the latter. |
Yeah really good point as well - especially as browser support for the latest features increases (right now the base is ES5, when do we move forward or always provide everything, etc). Maybe you'd want an ES5 base, or compiled down to latest spec version as well. There's also the idea of transpiling libraries/node_modules from the application itself but not really sure it's a good idea (yet)? |
Mulling all of what you guys have mentioned leads me to feel some sort of some sort of package.json level indication (polyfills field) could be a pretty neat idea. webpack already uses this similar concept when dealing with node built-in to provide api mocks on demand (but configured). I feel we could have a package of the most common 40 builtins and only the ones that are needed via a "polyfills" field. That's just an idea in my stream of consciousness atm. |
@Rich-Harris makes a point that it could be more about having an alternate dist that is sans-polyfill, just like lib authors ship builds sans-transpiled. |
I tend to agree with all points. So to recap:
I feel 4 may be too ambitious, because I find that in many of my codebases I'm using early-stage language features, or idiosyncratic ad-hoc stuff that I modify during the build process. Separating that from "untranspiled" may be tricky. That said, I would love to bikeshed on what the shape of this could look like. 😃 Maybe something like: // unpolyfilled
var library = require('library/bare');
import library from 'library/bare'; // es modules
// untranspiled, unpolyfilled
var library = require('library/raw');
import library from 'library/raw'; // es modules
// kitchen sink
var library = require('library');
import library from 'library'; // es modules
|
Because of "pragmatism > ideology" you end up with 2MB of JavaScript on a website. Those 5KB per module pile up.
I'd go for |
I once shipped a version serialport that accidentally polyfilled So I'm inclined to agree to not polyfill. And have shipped "bloated" libraries that might have used object.assign and promises but didn't share them and polute the global objects. |
I tend to not include polyfills. How should bundlers optimize different versions of the same polyfills? What if you always shipped your code without polyfills and provided a seperate polyfills.js file (named like that by convention) with all needed polyfills. That way your code stays lean and accessible while providing a clear path for users. My perceived main use cases are:
Bundlers could find this polyfill.js file via various means (package.json property, config file, etc.) and optimize for use cases 1 to 3. |
Frequently the
I think everyone agrees polluting globals is bad; what I'm talking about is "ponyfills," i.e. polyfills that don't pollute the global scope. :)
Unfortunately this pollutes the global scope, leading to problems like @reconbot describes.
The current method for inserting polyfills (e.g. used by babel-plugin-transform-runtime) is to identify global variables like |
Webpack 2 has moved to a This seems like the sanest course to me; polyfill requirements will not be the same between users so trying to blanket solve the problem on the module side seems like a fool's errand. |
@nolanlawson & @reconbot yeah, you both convinced me that my proposal isn't helpfull. I actually never ran into a case of multiple totally different polyfills myself, but since I used both @tomccabe so what you propose is that users that don't use a module bundler like webpack get all polyfills in the generated bundle and those that use bundlers build libs from source and thus provide all polyfills themselves, right? What about Typescript (or any other compiled language)? The build process gets more complicated when you build node_modules yourself. Do I also describe this compiled language in the Sorry if I'm not being helpfull. I'm still trying to sort this whole topic and somehow always end up running in circles. Provide polyfills or document which ones are needed? In the beginning I did bundle them. Nowadays I prefer the latter, but maybe I need to reconsider again? |
@nolanlawson but that's the point. That src/index.ts IS the untranspiled source you were talking about. Whether that's compatible with the user's transpiler is the user's problem; if there's a JS file with async/await in there and my transpiler is not new enough to support it, my build is still gonna fail. |
@nolanlawson I like your proposal with regards to providing users a choice between "raw" code and transpiled and polyfilled, "kitchen sink" code. Two questions I have though:
|
Just want to give my 2 cents. I think polyfills should be not part of the library. It's the job of the application to include polyfills.
|
I think the reality is that there isn't a one-size-fits-all answer — whether or not a particular library should include particular polyfills is always going to be a matter of judgment, and all we can really do is develop some conventions, and perhaps some shared guidelines about when and why to use them. For example, does it make sense for libraries to bundle their own But what about something like Then there's things like function getJSON ( url ) {
return fetch( url ).then( r => r.json() );
} I could add some documentation insisting that the app developer include the fetch polyfill, which has a lot of stuff I'm not using, and which doesn't even provide a lot of the good stuff (since
That can be true, though a library might have many more contributors, and updates benefit all their consumers. Also, we have good mechanisms for updating libraries (
Something I often observe in these sorts of discussions is that people like us, who spend our free time contributing to threads like this, are the kinds of experts who frequently underestimate what a colossal burden this stuff really is for a lot of developers, especially novices. Because those people are unlikely to take parts in these conversations, we have to consider their needs on their behalf. I think that it's an important principle, especially in web development, that the burden of extra configuration etc should fall on those most able to bear it. |
@bfred-it Hmm you may be right; a simple @georgezzhang I think you're right,
👏 👏 👏 This is why I don't think "raw by default" is feasible for most library authors. When you're inundated with bug reports from confused users, the path of least resistance is to just ship the polyfills and be done with it. Also, libraries that "work out of the box" tend to rise to the top because not every user will go the extra mile to understand a library with a long, complicated README. I still think the most interesting solution is a standard directory for "raw" code, so that bundlers/transpilers are aware of each library's unpolyfilled/untranspiled version. As @sokra says, if polyfilling is an app concern, then app bundlers could help users understand which global polyfills they would need in order to trim down their bundle size. E.g. imagine an interactive console in Webpack like:
(Webpack then parses dependencies, finds
Of course I'm totally making this up, and it would require a huge coordination between Webpack, Babel, and library authors to work. But maybe it's possible? |
haha @nolanlawson had a hard time understanding the shorter names
so in this scenario
webpack reads the the dependencies/node_modules (via some method) to determine the libraries/code that support this functionality and then uses those to compile. Sounds good? Basically either way we need to have multiple targets in npm packages (one for "raw" code which includes polyfills/transpiled) and one that includes everything done for you (easy to use). And we want the easy to use to be default. Thus a tool to make it happen |
Or we go raw-by-default and communicate in package.json which polyfills are needed. The build tool can handle adding these polyfills. This way it's easy to use and minimal in size. // package.json
"module": "src/index.js",
"uses": [
"Object.assign",
"fetch",
"Object.defineProperty"
] A shared module which contains default mappings for the polyfills is used by the build tool by default, and app developer could override/disable polyfills if needed. Maybe defaults are choosen by environment (i. e. |
Great discussion. Where did it move to? |
Nowhere for now... 😕 |
I very much appreciate all of the thoughts here, and the document that was updated for library considerations. One point I would like to add to this discussion (albeit late) is that as a library author you have absolutely NO idea what your target env is going to be. I can attempt to apply polyfills to my library (with "non polluting" due diligence), however that is from a perspective of the potential targets that may be problematic. I may be completely unaware of an antiquated target that requires a polyfill. In reality, we want babel to free us from this burden, and it is becoming better and better at doing this! From an app perspective, applying the needed polyfills has become a fairly simple and straightforward process, with the advent of the "babel-polyfill" import, in conjunction with babel's "babel-preset-env" configured with useBuiltins:"usage". That is why my opinion has progressed to the point of NOT applying polyfills at the library level, and merely documenting the potential need for the app to do this (depending on it's target env). Thoughts? |
My take on this is to give the means to install the polyfills that might be necessary to the user (e.g. package.json dependency field) and only then explicitly require the mandatory ones out of these (e.g. |
1688: Remove cross-fetch dependency r=brunoocasali a=flevi29 # Pull Request ## Related issue Fixes #1658 ## What does this PR do? - removes `cross-fetch` dependency - if anyone might need the polyfill they should run it themselves - there's an interesting discussion [here](w3ctag/polyfills#6 (comment)) about polyfills in libraries; I think, as a lot of people do, that the sane and clean path ahead is to make it the responsibility of the library user to polyfill, even if it's somewhat of a burden ## PR checklist Please check if your PR fulfills the following requirements: - [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)? - [x] Have you read the contributing guidelines? - [x] Have you made sure that the title is accurate and descriptive of the changes? Thank you so much for contributing to Meilisearch! 1698: Add `"packageManager"` entry to package.json r=brunoocasali a=flevi29 # Pull Request ## Why? - [`corepack`](https://nodejs.org/docs/latest-v20.x/api/corepack.html) [started adding this field automatically](nodejs/corepack#413), which can be disabled, but everyone has to do it separately, which is very annoying - anyhow there's no harm in it, `corepack` recommends it even in the "how to disable" doc https://github.com/nodejs/corepack/blob/main/README.md#environment-variables - > it ensures that your project installs are always deterministic (if you use `corepack`) - this does mean that this field will have to be updated periodically, but this is the recommended way ## PR checklist Please check if your PR fulfills the following requirements: - [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)? - [x] Have you read the contributing guidelines? - [x] Have you made sure that the title is accurate and descriptive of the changes? Thank you so much for contributing to Meilisearch! Co-authored-by: F. Levi <55688616+flevi29@users.noreply.github.com>
1688: Remove cross-fetch dependency r=brunoocasali a=flevi29 # Pull Request ## Related issue Fixes #1658 ## What does this PR do? - removes `cross-fetch` dependency - if anyone might need the polyfill they should run it themselves - there's an interesting discussion [here](w3ctag/polyfills#6 (comment)) about polyfills in libraries; I think, as a lot of people do, that the sane and clean path ahead is to make it the responsibility of the library user to polyfill, even if it's somewhat of a burden ## PR checklist Please check if your PR fulfills the following requirements: - [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)? - [x] Have you read the contributing guidelines? - [x] Have you made sure that the title is accurate and descriptive of the changes? Thank you so much for contributing to Meilisearch! Co-authored-by: F. Levi <55688616+flevi29@users.noreply.github.com>
I loved reading this document! One thing I found missing, though, was advice for library authors who use polyfills.
For instance, consider PouchDB. We ship various polyfills as part of the library:
Promise
,Map
,Set
,Object.assign
, etc. In each case we do the "right thing" – we prefer the native implementation, and we ship the polyfill in such a way that it doesn't overwrite any globals (i.e. a "ponyfill").But imagine that someone is using PouchDB, as well as Library B, Library C, and Library D, each of which does the same thing and ships their own polyfills for
Promise
,Map
,Set
… We could avoid a lot of this bloat if each library merely shipped code using the barePromise
,Map
,Set
, etc., but it's difficult to do so in such a way that doesn't aggravate users who want something that "just works."So most library authors seem to take the path of least resistance, and just ship their own polyfills to avoid any burden on their users. Hence: library bloat.
Now, I'm sure PouchDB could ship two versions of its codebase using some Babel/Rollup/whatever transform to inject polyfills for one version but not the other, and we could advise our advanced users to use the bare version and then Bring Your Own Polyfill, but this is a lot of extra effort and I fear nobody would do it unless 1) there were clear instructions for the various module bundlers – Browserify/Webpack/Rollup/Closure/RequireJS/etc. – on how to do this correctly, and 2) a large number of library authors actually got on board and did it. It may also require some kind of ad-hoc standard, e.g. a
"polyfills"
field inside ofpackage.json
laying out exactly which globals are required. I dunno.Without asking folks who are more familiar with bundlers/transpilers than I am, I'm not sure what advice exactly to give. But it may be worthwhile to at least acknowledge the problem in the document. (Unless you have some ideas for how to crack this nut, which I don't. 😃)
/cc @TheLarkInn @hzoo @Rich-Harris who have probably thought about this a lot more than me.
The text was updated successfully, but these errors were encountered: