added routing module #382

gavinr · 2018-11-07T14:47:14Z

Beginning of the routing module and the the solveRoute function, covering the solve endpoint of the REST API.

coveralls · 2018-11-07T14:59:52Z

Coverage remained the same at 100.0% when pulling 207eaab on gavinr:routing-module into ee76650 on Esri:master.

jgravois

awesome work @gavinr! 👨‍🚀

jgravois · 2018-11-07T16:52:23Z

packages/arcgis-rest-routing/README.md

+```
+
+```js
+todo


~~to do~~

solveRoute({ stops: [ [-117.195677, 34.056383], [-117.918976, 33.812092], ], authentication }) .then(response) // {routes: {features: [{attributes: { ... }, geometry:{ ... }}]}}

jgravois · 2018-11-07T16:52:27Z

packages/arcgis-rest-routing/README.md

+
+### License
+
+Copyright &copy; 2017-2018 Esri


gotta start at 2018 for this one.

jgravois · 2018-11-07T16:52:31Z

packages/arcgis-rest-routing/package.json

@@ -0,0 +1,54 @@
+{
+  "name": "@esri/arcgis-rest-routing",
+  "version": "1.0.0",


1.13.2 for now. lerna will help us bump to 1.14.0 for the first actual release.

jgravois · 2018-11-07T16:53:29Z

packages/arcgis-rest-routing/src/helpers.ts

@@ -0,0 +1,23 @@
+/* Copyright (c) 2017 Environmental Systems Research Institute, Inc.


jgravois · 2018-11-07T16:53:54Z

packages/arcgis-rest-routing/src/solveRoute.ts

@@ -0,0 +1,124 @@
+/* Copyright (c) 2017-2018 Environmental Systems Research Institute, Inc.


jgravois · 2018-11-07T16:56:20Z

packages/arcgis-rest-routing/src/solveRoute.ts

+
+export interface ISolveRouteRequestOptions extends IEndpointRequestOptions {
+  stops: Array<IPoint | ILocation | [number, number]>;
+  travelMode?: string;


lets nix travelMode and barriers until we're actually handling them internally.

jgravois · 2018-11-07T16:57:59Z

packages/arcgis-rest-routing/src/solveRoute.ts

+} from "./helpers";
+
+export interface ISolveRouteRequestOptions extends IEndpointRequestOptions {
+  stops: Array<IPoint | ILocation | [number, number]>;


you can see an example of how to include inline TSDoc for an interface property like stops here:

arcgis-rest-js/packages/arcgis-rest-geocoder/src/helpers.ts

Lines 19 to 22 in 4af2014

/**

* Any ArcGIS Geocoding service (example: http://sampleserver6.arcgisonline.com/arcgis/rest/services/Locators/SanDiego/GeocodeServer )

*/

endpoint?: string;

jgravois · 2018-11-07T17:00:33Z

packages/arcgis-rest-routing/src/solveRoute.ts

+ *   });
+ * ```
+ *
+ * @param address String representing the address or point of interest or RequestOptions to pass to the endpoint.


@param requestOptions Options to pass through to the routing service.

jgravois · 2018-11-08T00:35:28Z

i decided to kill two birds with one stone and reorganize a few of the utility methods and interfaces you copied from the 'geocoder' package and put them in 'common-types' so they can be shared instead.

this should resolve the problem @MikeTschudi reported in Esri/solution.js#3 (comment) as well.

can you sanity check me please @tomwayson?

patrickarlt · 2018-11-08T17:14:22Z

@jgravois should we keep common-type for just types and make a separate shared-utils package for shared utility methods?

tomwayson · 2018-11-08T17:27:14Z

I definitely think we should not have fns in common-types.

I really like what @jgravois set up for hub.js where there is a "common" package that has types and fns, and I think for rest-js v2.0 we should do exactly that.

I understand why it would seem to make sense to just start using common-types like that now, but the problem is that it's name suggests, and up until now our convention has been, that it's an optional dependency that you only need if you're using TS. I don't think it's a good idea to all of the sudden make it actually required in some cases.

In the meantime, I think we should go ahead and create @esri/arcgis-rest-common, and start by moving these fns there. We could even move the types there too before a v2 release and deprecate the entire @esri/arcgis-rest-common-types.

jgravois · 2018-11-08T19:37:57Z

I think we should go ahead and create @esri/arcgis-rest-common, and start by moving these fns there.

that works for me. i'd be more inclined to go with @patrickarlt's suggestion but the types are already automatically stripped from built umd output and i'm hopeful we could do the same in cjs. if so, and we can make sure only d.ts files are created for types in the esm build, it'd make for another tidy little package.

to do:

revert the changes in my last commit and move the shared functions into a second new package called @esri/arcgis-rest-common
make sure routing includes common as a dependency and not common-types
refactor geocoder to use common at v2.0.0
figure out how to help @MikeTschudi with his unrelated problem

tomwayson · 2018-11-08T19:52:48Z

just curious, why not refactor geocoder to start using the fns in 'common' now? is that breaking?

jgravois · 2018-11-08T20:10:00Z

why not refactor geocoder to start using 'common' now? is that breaking?

exactly.

…ND types AFFECTS PACKAGES: @esri/arcgis-rest-common @esri/arcgis-rest-geocoder @esri/arcgis-rest-routing

jgravois · 2018-11-09T22:32:27Z

okay, i finished refactor (2) and rebased to clean up my own tail of the commit history.

tomwayson

I think this is the start of something beautiful

added routing module

157e649

gavinr requested a review from jgravois November 7, 2018 14:47

jgravois suggested changes Nov 7, 2018

View reviewed changes

fixed documentation and cleaned up code

d5d4ef0

jgravois approved these changes Nov 7, 2018

View reviewed changes

jgravois requested a review from tomwayson November 8, 2018 00:35

patrickarlt mentioned this pull request Nov 8, 2018

Param builders for building complex parameter objects #384

Open

feat(:file_folder:): new generic common package for utility methods A…

0bd3ea7

…ND types AFFECTS PACKAGES: @esri/arcgis-rest-common @esri/arcgis-rest-geocoder @esri/arcgis-rest-routing

whoops, missed one

207eaab

tomwayson approved these changes Nov 14, 2018

View reviewed changes

jgravois merged commit eeafb6b into Esri:master Nov 14, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added routing module #382

added routing module #382

gavinr commented Nov 7, 2018

coveralls commented Nov 7, 2018 •

edited

Loading

jgravois left a comment

jgravois Nov 7, 2018

jgravois Nov 7, 2018

jgravois Nov 7, 2018

jgravois Nov 7, 2018

jgravois Nov 7, 2018

jgravois Nov 7, 2018

jgravois Nov 7, 2018

jgravois Nov 7, 2018

jgravois commented Nov 8, 2018

patrickarlt commented Nov 8, 2018

tomwayson commented Nov 8, 2018 •

edited

Loading

jgravois commented Nov 8, 2018 •

edited

Loading

tomwayson commented Nov 8, 2018

jgravois commented Nov 8, 2018

jgravois commented Nov 9, 2018

tomwayson left a comment

+              ```
+              ```js
+              todo

		@@ -0,0 +1,23 @@
		/* Copyright (c) 2017 Environmental Systems Research Institute, Inc.

		@@ -0,0 +1,124 @@
		/* Copyright (c) 2017-2018 Environmental Systems Research Institute, Inc.

	/**
	* Any ArcGIS Geocoding service (example: http://sampleserver6.arcgisonline.com/arcgis/rest/services/Locators/SanDiego/GeocodeServer )
	*/
	endpoint?: string;

added routing module #382

added routing module #382

Conversation

gavinr commented Nov 7, 2018

coveralls commented Nov 7, 2018 • edited Loading

jgravois left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jgravois commented Nov 8, 2018

patrickarlt commented Nov 8, 2018

tomwayson commented Nov 8, 2018 • edited Loading

jgravois commented Nov 8, 2018 • edited Loading

tomwayson commented Nov 8, 2018

jgravois commented Nov 8, 2018

jgravois commented Nov 9, 2018

tomwayson left a comment

Choose a reason for hiding this comment

coveralls commented Nov 7, 2018 •

edited

Loading

tomwayson commented Nov 8, 2018 •

edited

Loading

jgravois commented Nov 8, 2018 •

edited

Loading