search

Esri / hub.js

TypeScript wrappers for talking to ArcGIS Hub
https://esri.github.io/hub.js/
Apache License 2.0
33 stars 13 forks source link

consolidate packages #306

Open tomwayson opened 4 years ago

tomwayson commented 4 years ago

Seems to me that there is unnecessary friction in the development and consumption of this library due the large number of packages.

The strategy of diving up the library into many micro-packages that are peer depenedencies came from rest-js. When we started work on that, tree shaking and code splitting were less reliable, and not available to the Ember apps that were going to use the packages. Neither of those are true any longer. We can now assume that most consumers of this library will use the ESM builds and be able to tree-shake out just the code that their apps need.

I think we could reduce the number of packages by about half if we:

I think packages like events and surveys should independent for now at least.

tomwayson commented 3 years ago

@cpgruber what would you think about consolidating hub-surveys into hub-content?

@ajturner ultimately I could see a hub-feedback package that replaces hub-annotations and includes all that code and it would depend on the @hub-content package for survey related functions.

cpgruber commented 3 years ago

@cpgruber what would you think about consolidating hub-surveys into hub-content?

@ajturner ultimately I could see a hub-feedback package that replaces hub-annotations and includes all that code and it would depend on the @hub-content package for survey related functions.

I would defer to @rweber-esri and @brollison

brollison commented 3 years ago

@cpgruber I don't believe I have enough expertise here to comment in a helpful way; however, if a package called hub-feedback is meant to exist, then hub-surveys may be better suited there over hub-content. There is a lot we do or plan to do with surveys which treats them differently from most other content, but I'm not sure if that means they need to be separated from more generic content items.

// @rweber-esri please be more helpful / constructive than me ;-)

rweber-esri commented 3 years ago

@tomwayson

It may help to have a little more context into what the friction is specifically. Is the friction that people are having difficulty finding what they need or more along the lines of build/version bump pains (suspect the latter)? My main concern about consolidating the different type-specific packages into a more generic content package is centered around exporting & collisions. We do a lot of export * from "../wherever"; and could have some collisions that would force us to rename functions and/or add an abstraction layer like solutions-service in opendata-ui that delegates.

tomwayson commented 3 years ago

@rweber-esri

more along the lines of build/version bump pains (suspect the latter)

correct, though there's also pain in having to consume so many peer dependencies.

re: exporting and collisions, that's already an issue since our UMD builds put all the fns into a single namespace (arcgisHub).

tomwayson commented 3 years ago

My takeaway from this discussion is that we should essentially rename @esri/hub-annotatinos as @esri/hub-feedback and we can move the more specific code from @esri/hub-surveys into that new package and possibly the more generic code from @esri/hub-surveys (if any) into @esri/hub-content.

tomwayson commented 3 years ago

I've recently noticed that tree shaking of at least the @esri/hub-common package doesn't work in a new ember app.

I installed @esri/hub-common and it's peerDependencies and then added this line to the application controller: import { camelize } from '@esri/hub-common';. When I run ember s I see that there are hundreds of eval() statements, one for each ESM module in @esri/hub-common, and for the modules it depends on from arcgis-rest-js.

I think we ought to verify that tree shaking of these libraries works outside of ember-auto-import before too much further consolidation of pacakges.

tomwayson commented 3 years ago

Well the members package currently only exposes comingSoon() so it's not even doc'd:

image

So it's just there to slow down build times.

tomwayson commented 2 years ago

Here's my revised proposal on how to keep this moving along.

Frist, we do the no brainers:

Next, we address #655, which makes dependency management from solution.js and template-js easier.

Then, we address #656, which makes dependency management for hub-components easier.

That will leave us with the following medium to large packages, but we should wait until we have our lazy loading strategy (i.e. #517) in place before merging them into common: