Skip to content

Reorganize components/ and lib/ #3271

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

Closed
alexcjohnson opened this issue Nov 20, 2018 · 1 comment
Closed

Reorganize components/ and lib/ #3271

alexcjohnson opened this issue Nov 20, 2018 · 1 comment

Comments

@alexcjohnson
Copy link
Collaborator

Sparked by a comment by @etpinard #3254 (comment):

It might be nice to make a distinction between "registerable" (i.e optional) component modules (e.g. annotations/, shapes/) and component modules that are mostly intended to DRY things up (e.g. color/, colorscale/ ... and the soon-to-be axes/). Maybe the latter could be placed in different directory e.g. src/common/ and leave src/components/ for "registerable" modules?

Completely agree we should be clearer about what components/ means. My gut reaction is actually to go even further: common/ serves the same role as lib/, and lib/ itself has perhaps outgrown its origin as a flat directory that exports everything in one namespace. In fact it already doesn't do this, these files are in lib/ but not incorporated in lib/index:
clear_gl_canvases, events, geo_location_utils, geojson_utils, gl_format_color, gup, override_cursor, polygon, prepare_regl, queue, show_no_webgl_msg, str2rgbarray, svg_text_utils, topojson_utils

So my proposal:

  • lib/index should only contain small functions that have no dependencies on anything else in our codebase (except constants/; currently it also imports d3 and fast-isnumeric, those seem fine), so it can be safely imported by anyone including other files inside lib/, and it should not import anything else from lib/. This would require most consumers of lib/ to pull in the individual pieces they need rather than just require('../../lib') - that's probably the most controversial part of this proposal, as it would be a bunch of work, but I think it would be an improvement in terms of being explicit about imports, and it would let us 🔪 dumb files like lib/mod and lib/noop that exist only so other lib consumers can import them. It could also reduce the size of partial bundles a little bit.
  • We move unregisterable "components" to be subdirectories of lib/
  • We try to group other files in lib into subdirectories too - in fact some of the above-mentioned lib files could go into components/color -> lib/color - but I also don't see anything wrong with leaving some hard-to-categorize files directly in lib/.
@gvwilson
Copy link
Contributor

Hi - this issue has been sitting for a while, so as part of our effort to tidy up our public repositories I'm going to close it. If it's still a concern, we'd be grateful if you could open a new issue (with a short reproducible example if appropriate) so that we can add it to our stack. Cheers - @gvwilson

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