mjackson / unpkg

The CDN for everything on npm
https://unpkg.com
Other
3.01k stars 303 forks source link

RFC: Remove support for `browser` field and ?main at "bare" URLs #63

Open mjackson opened 7 years ago

mjackson commented 7 years ago

Currently, if someone requests the "bare" URL (a URL w/out a filename) for a package with a browser field in its package.json, that file is the one that gets served. For example, when we see a request for /d3 (no filename), since d3 defines { browser: "build/d3.js" } in its package.json we redirect to https://unpkg.com/d3/build/d3.js.

I originally introduced this behavior because I wanted to enable people publishing browser-ready files in browser to be able to have super clean URLs (e.g. <script src="//unpkg.com/d3">), but there's a subtle problem: we have a different interpretation of browser than the bundlers do.

This was actually brought up in #25 but for some reason I didn't quite understand the problem at that time. Probably because I hadn't done much work with ES modules yet.

Scenario 1: Imagine a package with the following config:

{
  "main": "index.common.js",        // node, Browserify, unpkg bare URL
  "module": "index.es.js"           // rollup, webpack2, <script type=module>
}

In this scenario, everything works great:

You could say it's not perfect because the unpkg bare URL points to a CommonJS module, but we'll set that aside for now.

Scenario 2: Now, let's say you have a browser-specific shim for some of your code, so you add the browser field to swap out the node-specific index.common.js module with browser-ready one: index.browser.js:

{
  "main": "index.common.js",        // node
  "browser": "index.browser.js",    // Browserify, unpkg bare URL
  "module": "index.es.js"           // rollup, webpack2, <script type=module>
}

Now:

The problem is that the unpkg "bare" URL moved as well. So the package author just wanted a way to introduce some browser-specific shims, but we actually changed the way we serve the package too. But the browser field was never intended as a place to put your UMD build.

Scenario 3: To get around this problem we added support for the unpkg field, so you could do:

{
  "main": "index.common.js",        // node
  "browser": "index.browser.js",    // Browserify
  "unpkg": "index.umd.js",          // unpkg bare URL
  "module": "index.es.js"           // rollup, webpack2, <script type=module>
}

So this is good because now at least people who want to use the browser field for what it was originally intended for (browser shims) also have a way to override which file is served at the bare URL on unpkg.

But this also makes the fact that we fall back to using the browser field (when there is no unpkg field) completely redundant!

Basically all we need to do is this: if you want to make unpkg serve something other than main at the bare URL, use the unpkg field. That's it. We don't even need a ?main query parameter and we never need to do anything with browser.

Anyway, if we did remove support for ?main and the browser field it would definitely be a breaking change, so I'd be happy to redirect any old URLs that relied on this behavior to minimize the impact. But going forward it seems like this is what the default should actually be. I hate to change this kind of thing because I know it's a pain to update stuff, but I think this should be one of the last breaking changes. The biggest win really would be that the whole service is a little less surprising and easier to understand for newcomers.

ping @mbostock @ngokevin @charlike @Daniel15 @ggoodman @EricSimons

For related discussions, see:

ngokevin commented 7 years ago

No opinion about the deprecation/legacy paths, but an unpkg field is exactly what I've wanted. I ran into issues before I think with overloading the browser field which changes the behaviors of bundlers.

mjackson commented 7 years ago

an unpkg field is exactly what I've wanted

@ngokevin 👍 Awesome, I thought so. We currently support the unpkg field (just wanted to make that clear in case it wasn't in the OP)

Andarist commented 7 years ago

Ill paste in relevant discussion from twitter - not long time ago - https://twitter.com/AndaristRake/status/900717992165728257 , as it seems relevant

Me: I have a slight beef with pkg.browser. Aint using it because webpack tries to load it by default and i expect my code to be bundled mostly Me: So i just gave up having nice unpkg url and distribute umd build in npm but without specifying browser field @Rich-Harris: Not sure I follow — it's still better for webpack to use your prebundled file, no? Or do you mean your UMD contains 3rd party deps? Me: My umd is es5 with umd wrapper, so its not tree-shakeable for instance atm. Also i want webpack users to benefit from dev code during dev @Rich-Harris: makes sense 👍

Actually I had no idea that I can specify unpkg entry in my package.json, so it seems that my whole problem is gone now once I know about it ;) But certainly fallbacking to the browser field got me confused and caused me some headaches in the past.

Glad to see it changing.

jesstelford commented 7 years ago

Any stats on the number of cases where the browser fallback is kicking in? (Not just packages that have the field, but actual loads of the file via unpkg)

I'm 👍 for the change, but a breaking change in a CDN could have serious unknowable consequences / side effects.

mbostock commented 7 years ago

I support this change and I’d be willing to push 30+ 53+ EDIT: 6 releases to make it happen. Though I’m not sure what the backwards-compatibility strategy is for old releases? Are you going to compile a list of existing package@version that depend on this behavior, or are you going to pick a cutoff date by package release that switches to the new behavior?

This will fix a frequent issue webpack users experiencing consuming D3 as ES modules: webpack’s resolve.mainFields defaults to ["browser", "module", "main"] when the target is web, so by default webpack users get the pre-built UMD bundle rather than the ES modules.

tunnckoCore commented 7 years ago

Finally. I'm happy to see that change and that you reconsidering it. I believe there's no one which expect and depends on that behavior of unpkg. The browser field, may not be intended to serve UMDs, but it is actually used for this in most cases, imho. And it make sense, actually, and it's correct CDNs like UNPKG to rely on it. But it really not play well with bundlers and what they think for and how they use the browser field.

As about the unpkg field... i didn't know that too. When this change landed? Was it mentioned in the site? I believe it wasn't supported when i was struggling. But anyway.

Actually the query param ?main is good idea and may remain.

Said shorter than #25, the whole problem is that UNPKG expect browser field to be something that can be served in the browser directly without anymore job to do (so in most cases that is UMD build), but Browserify in another hand expect the file pointed in browser field to be something that it will bundle. Yes, browserify can bundle UMD, of course, but it not make much sense and adds extra bytes, because both UMD and browserify wrappers are almost the same.

Anyway. Good move, great to hear that :tada: :)

mjackson commented 7 years ago

Are you going to compile a list of existing package@version that depend on this behavior, or are you going to pick a cutoff date by package release that switches to the new behavior?

I think the cleanest solution will probably be to pick a date after which unpkg will not use the browser field for any package published after that date. That should give package authors enough time to migrate to the unpkg field if they need to while still allowing all old URLs to work.

Since there seems to be a general consensus that this is a good direction to take, I'd suggest we set the date for Oct 1, 2017. I'll post some instructions for package authors to the site next week and that should give people enough time to make whatever adjustments they need.

As for the ?main query parameter, we can probably continue to support that indefinitely for the sake of existing URLs, though I will consider it deprecated and won't be encouraging anyone to use it in the future.

Finally.

Ya, I know. I can be a little slow sometimes ;)

When this change landed? Was it mentioned in the site?

IIRC I added support for the unpkg field very soon after we last talked in #25, @charlike. It's currently mentioned on the site but it could be much more prominent. I'll make that adjustment this next week as well :)

tunnckoCore commented 7 years ago

though I will consider it deprecated and won't be encouraging anyone to use it in the future.

I think it is good feature and should remain in future. It gives flexibility and in bonus, it will make the breaking change (the fact of dropping browser field support) transition smoother.

mbostock commented 7 years ago

jsDelivr is proposing a cdn field as a neutral equivalent to unpkg and jsdelivr. Not sure if there is enough momentum yet to declare that a standard, but from a maintainer perspective it would be nice to have a neutral name if everyone can agree on it. Of course that was the original goal with browser so it is hard to know if cdn is specific enough to avoid ambiguity.

mbostock commented 7 years ago

(To be clear, if you do support cdn I would do that in addition to unpkg since that is already in use.)

bryanculver commented 7 years ago

This RFC (or rather D3 moving to adhere to it) broke something with my Rails app using D3 regarding Schmooze. After reviewing the RFC I am in favor of it, just want to note that I detailed my issues here: https://github.com/d3/d3/issues/3138#issuecomment-326813230.

I will be opening an issue with a hopeful pull request to resolve the issue, as soon as I can find the proper place to fix this.

mjackson commented 7 years ago

I think it is good feature and should remain in future.

@charlike Can you please say more? I can't see any scenario in which ?main would be useful once we get the default behavior right for the bare URLs. ?main=browser was the only real use case I could think of when I introduced it, and then some people complained that I should just default to using browser so they wouldn't have to use ?main, so it's kind of the reason I ended up in this mess in the first place. I'd like to get rid of it unless there's a solid use case for it. Just seems like extraneous API at this point, which is always bad.

jsDelivr is proposing a cdn field as a neutral equivalent to unpkg

@mbostock Since the unpkg field is tied directly to our implementation of the "bare" URLs, I'd rather stick to using that for now. There is no consensus at this point for what a cdn field would mean, and if we decide to enhance the meaning of the unpkg field then we'd have to clear it with them first. There's still a lot of work to do, so I'd like to retain creative control over that field for at least a little while longer so we can continue to do awesome stuff with it :)

Of course, jsDelivr is free to support the unpkg field directly instead of introducing their own, but that's up to them. I've tried working together with them in the past, but it hasn't gone very well...

tunnckoCore commented 7 years ago

@mjackson don't have very strong and specific case. Just thinking when everything is good soon, it won't be a problem to have support for this. This only strong case is smoother transition, at least.

As about the cdn field, sounds good. I don't know where and when the browser was introduced, and by who, but i believe Browserify? For first time i seen it there before few years. Don't know.

Daniel15 commented 7 years ago

jsDelivr is proposing a cdn field as a neutral equivalent to unpkg and jsdelivr. Not sure if there is enough momentum yet to declare that a standard, but from a maintainer perspective it would be nice to have a neutral name if everyone can agree on it.

I agree with this. The field is a general reusable concept ("when this package is loaded via HTTP, which file should be served?) that's not specific to Unpkg at all, so it makes more sense to give it a more general name.

mjackson commented 7 years ago

when this package is loaded via HTTP

There are numerous ways to "load a package via HTTP". Currently, people can (and do) load packages from unpkg using:

It may seem like there is a general consensus in the JS ecosystem on how to publish and consume code, but that's just not the case, especially when it comes to consuming code from npm over HTTP which is a fairly recent idea.

Also, as I mentioned above, I'm fairly certain that we'll expand the definition of the unpkg field in the future to include other directives that are specific to unpkg.

Daniel15 commented 7 years ago

Good points. I didn't even consider the newer stuff like Githubissues.

  • Githubissues is a development platform for aggregating issues.