search

palantir / plottable

:bar_chart: A library of modular chart components built on D3
http://plottablejs.org/
MIT License
2.98k stars 221 forks source link

Don't commit built plottable.js file #3212

Closed hellochar closed 7 years ago

hellochar commented 7 years ago

Remove the built file from our committed files since it adds noise to PRs, causes git conflicts, etc.

This will break existing users who pull us in through a script tag. To unbreak them, we should publish every dev build (#3308), which will include the built .js file, and recommend users reference that through https://unpkg.com/.

handrail: replace plottable.js with a script that emits a console warning that tells people how to upgrade.

adidahiya commented 7 years ago

I don't think you need to remove the standalone bundle from the plottable distribution (we just added it to blueprint in https://github.com/palantir/blueprint/issues/303), but I agree it shouldn't be checked into the repo.

hellochar commented 7 years ago

I see, makes sense. Are you guys also publishing dev builds to npm?

adidahiya commented 7 years ago

@hellochar no, not yet

hellochar commented 7 years ago

d3v4 ( #3091 ) complicates this since we currently don't bundle d3 with our standalone (expecting users to pull it in for us). Users doing <script src="d3js.org/3.5.16/d3.js"> <script src="plottable"> will break since we now rely on 4.0. Asking them to point to 4.0/d3.js is not enough since we use d3-selection-multi which isn't part of the standard bundle.

I think we should just change the semantics of standalone plottable.js to bundle all our dependencies (aka all of d3 and d3-selection-multi). This actually maintains some semblance of backcompat since Plottable's internal references to d3 will refer to the standalone bundled version, which is pretty cool. Regardless we'll need to message out to users how to use standalone in this new world.

adidahiya commented 7 years ago

Asking them to point to 4.0/d3.js is not enough since we use d3-selection-multi which isn't part of the standard bundle.

Is there any CDN-hosted bundle out there that has all the d3v4 packages included?

hellochar commented 7 years ago

Not anything officially supported. This makes sense; separate bundles begets itself to a "plugin" architecture, so it doesn't make sense for CDNs to hold combinations of different modules included/not included

themadcreator commented 7 years ago

by removing the standalone library, we mean from git, not from npm

hellochar commented 7 years ago

Blocked by #3308

adidahiya commented 7 years ago

why is this blocked by #3308?