arcgis-rest-js icon indicating copy to clipboard operation
arcgis-rest-js copied to clipboard

Param builders for building complex parameter objects

Open patrickarlt opened this issue 7 years ago • 6 comments

Given that this library now has the capacity to do routing https://github.com/Esri/arcgis-rest-js/pull/382 and create feature services I want to put forward the idea of including some reasonable APIs for building the parameters objects like what I specced for https://github.com/Esri/esri-leaflet-routing/blob/master/Route.md and https://github.com/Esri/esri-leaflet-routing/blob/master/TravelArea.md. THis would make the job of building out the params or options objects much more reasonable for things like IAddToServiceDefinitionRequestOptions to wrap common use cases. For example:

import { addToServiceDefinition, layerBuilder } from '@esri/arcgis-rest-feature-service-admin';

const layer = layerBuilder()
  .field({/*...*/})
  .field({/*...*/})
  .field({/*...*/})
  .spatialReference(4326)
  .enableEditing();

addToServiceDefinition(serviceurl, {
  authentication: userSession,
  layers: [layer]
});

Opening this up for comment, @noahmulfinger @araedavis @tomwayson @dbouwman @jgravois.

patrickarlt avatar Nov 08 '18 17:11 patrickarlt

Having fluent builders would certainly help us bury more platform idiosyncrasies, so that alone is a win.

import { search, queryBuilder } from '@esri/arcgis-rest-items';

// ability to construct a template that can later be refined... 
// a sort of partial-application on an object graph
const texasWaterWebApps = queryBuilder()
   .withAuth(myAuthFromSessionOrWherever)
   .hasType('Web Mapping Application')
   .withoutType('Web Map')
   .hasTag('water')
   .hasTag('texas')
   .culture('en-us')
   .hasKeyword('hubSolution')
   .withoutKeyword('hubSolutionTemplate');

// later... given some user input...
const query = queryBuilder(texasWaterWebApps)
    .search('waco');

return search(query).then(...)

dbouwman avatar Nov 08 '18 20:11 dbouwman

i think this is a great idea.

jgravois avatar Nov 09 '18 20:11 jgravois

I like this idea.

I think it will be a bit challenging in TS. I wrote a fluent API for cedar and it was a lot of overloads juggling.

Also, I do think there could be a little cognitive dissonance if most of the fns in the lib are stateless fns that return promises, and then there are a few fluent (stateful) builders. I wonder how this will interplay w/ #339

All that said, I think this will provide a valuable benefit to consumers of this library.

tomwayson avatar Nov 12 '18 03:11 tomwayson

@tomwayson @dbouwman I'm really interested in writing at least the first one of these for building a search query parameter after running through the horror of https://github.com/Esri/arcgis-rest-js/blob/master/demos/node-cli-item-management/index.js#L101-L132.

Before I get too far into this though I know you both are (particularly @dbouwman) are huge FP fans. Fluent interfaces in TS would be super easy with classes but in keeping without goal of ideally NOT using classes and holding state in them I did some extra research.

  1. Something like https://hackernoon.com/writing-fluent-functional-code-384111431895 would be super cool but cant work without variadic types in TypeScript which look like their own special hell.
  2. Having builders be more factory oriented
    function QueryBuilder() {
      var params = { q: "" , num: 10, start: 0};
      return {
        // the type for this is actually optional here but it might add some type safety
        andTags(this: ReturnType<typeof QueryBuilder>, ...tags) {
          params.q += tags.join(",");
          return this;
        },
        toParams() {
          return params;
        }
      };
    }
    
    const params = QueryBuilder()
      .andTags("foo", "bar")
      .andTags("baz")
      .toParams();
    
    console.log(params);
    ```;
    
  3. We could always use classes:
    class QueryBuilder {
      private q: string = "";
      private num: number = 10;
      private start: number = 0;
    
      andTags(...tags) {
        this.q += tags.join(",");
        return this;
      }
    
      toParams() {
        return {
          q: this.q,
          num: this.num, // fetch from internal state
          start: this.start, // fetch from internal state
        };
    
      }
    }
    
    const params = new QueryBuilder()
      .andTags("foo", "bar")
      .andTags("baz")
      .toParams();
    
    console.log(params);
    

I'm really leaning towards option 2 here. I think it will be the easiest option.

In either case when request runs I think we should check to see if the value of requestOptions.params has a toParams() method. If it does we will call it and set the request params as the result.

To allow the builder not to have to warp EVERY param on the API (although ideally they should) we could allow an escape hatch in toParams to allow any additional params to be merged in.

function QueryBuilder() {
  var params = { q: "" , num: 10, start: 0};
  return {
    // the type for this is actually optional here but it might add some type safety
    andTags(this: ReturnType<typeof QueryBuilder>, ...tags) {
      params.q += tags.join(",");
      return this;
    },
    toParams(additionalParams) {
      return {...params, ...additionalParams};
    }
  };
}

const params = QueryBuilder()
  .andTags("foo", "bar")
  .andTags("baz")
  .toParams({someOtherParamThatIsntWrapped: true});

console.log(params);

patrickarlt avatar Apr 11 '19 17:04 patrickarlt

this will be included in v2.0.0. hopefully later today!

jgravois avatar Apr 19 '19 21:04 jgravois

Reopening this to continue to track discussion of additional builders.

patrickarlt avatar May 01 '19 18:05 patrickarlt