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

chore: pkg.browser > pkg.unpkg #421

Merged
merged 1 commit into from
Jan 7, 2019
Merged

chore: pkg.browser > pkg.unpkg #421

merged 1 commit into from
Jan 7, 2019

Conversation

jgravois
Copy link
Contributor

resolves #420 🚬

AFFECTS PACKAGES:
@esri/arcgis-rest-auth
@esri/arcgis-rest-common
@esri/arcgis-rest-feature-service-admin
@esri/arcgis-rest-feature-service
@esri/arcgis-rest-geocoder
@esri/arcgis-rest-groups
@esri/arcgis-rest-items
@esri/arcgis-rest-request
@esri/arcgis-rest-routing
@esri/arcgis-rest-sharing
@esri/arcgis-rest-users

AFFECTS PACKAGES:
@esri/arcgis-rest-auth
@esri/arcgis-rest-common
@esri/arcgis-rest-feature-service-admin
@esri/arcgis-rest-feature-service
@esri/arcgis-rest-geocoder
@esri/arcgis-rest-groups
@esri/arcgis-rest-items
@esri/arcgis-rest-request
@esri/arcgis-rest-routing
@esri/arcgis-rest-sharing
@esri/arcgis-rest-users

ISSUES CLOSED: #420
@jgravois jgravois changed the title chore(:package:): pkg.browser > pkg.unpkg chore: pkg.browser > pkg.unpkg Dec 18, 2018
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 994b5d7 on chore/unpkg into c1c9dd6 on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 994b5d7 on chore/unpkg into c1c9dd6 on master.

@tomwayson
Copy link
Member

wait, what? please 'splain. is this a thing?

@jgravois
Copy link
Contributor Author

is this a thing?

TL;DR

yes. if its good enough for D3, its good enough for me.

just TL

unpkg.com can be used to serve up any file in an npm package, but historically when none are specified explicitly in a url, it responded with whatever was specified in the browser field (or main as a fallback).

this led to complaints (see mjackson/unpkg#25) that this is an unnecessary collision with bundlers like browserify that would expect Common JS in that slot so now "unpkg" > "main" > "browser" and someday "browser" could be dropped.

post script. as of yesterday no automatic redirects are working because of a different bug (see mjackson/unpkg#156).

post post script. this is just trivia, but esri-leaflet plugins make up the overwhelming majority of unpkg traffic still relying on the browser fallback.

@tomwayson
Copy link
Member

OK, I missed the ref to #420 (I've only had 1 ☕️ ) and it's ref to mjackson/unpkg#63

So yeah, I think we should add unpkg, but not sure if we should remove browser, and if we don't, not sure what it should point to.

I'm having a hard time thinking of pkg.browser purely as a place to specify a bundler entry point that includes browser-only shims. It appears from that issue that's how browserify (R.I.P.) used it, but it's unclear to me if webpack does the same. There are other uses though. NPM says it's a flag to indicate that your code expects to be run in a browser, but unsurprisingly doesn't weigh in on whether it's expected to be UMD, or if dependencies are expected to be bundled.

In cases like this, I always ask, what would Bostock do, and it looks like you may have already asked that, b/c he doesn't include a browser field:

https://github.com/d3/d3/blob/3e2bbb506b536fdf13bfdf6a106d1c16b4859666/package.json#L18-L20

At the end of the day, I think we should keep browser as is if for no other reason than it's a breaking change, and I'm pretty sure some of our own Ember code relies on it.

@jgravois
Copy link
Contributor Author

jgravois commented Dec 19, 2018

jinx 😉.


I think we should keep browser as is if for no other reason than it's a breaking change, and I'm pretty sure some of our own Ember code relies on it.

AFAIK all our ember code points straight at umds using their explicit location.

the benefit of dropping browser is that it helps webpack users that want to consume ES Modules avoid the footgun bostock mentions here: mjackson/unpkg#63 (comment)

@jgravois jgravois merged commit b84bad2 into master Jan 7, 2019
@jgravois jgravois deleted the chore/unpkg branch January 7, 2019 19:49
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

Successfully merging this pull request may close these issues.

pkg.browser > pkg.unpkg
3 participants