Specify node engine version

morganherlocker commented 5 years ago

sharedstreets-js only runs on node.js v11, which is the current version. This needs to be specified with the engine parameter in package.json to avoid unexpected behavior for users of the library and CLI.

josiekre commented 5 years ago

I have node v8 installed locally on my Mac, so I thought I'd try to get a Docker going.

I tried building from node:11, but I'm not able to get it working:

FROM node:11

ENV NPM_CONFIG_PREFIX=/home/node/.npm-global
ENV PATH=$PATH:/home/node/.npm-global/bin

USER node
RUN npm install -g sharedstreets@0.12.0

$ docker build -t shst .
$ docker run -it --rm -v ~/Downloads:/usr/ shst:latest shst match /usr/map.geojson --out=/usr/match.geojson
standard_init_linux.go:207: exec user process caused "no such file or directory"

If I look inside...

$ docker run -it --rm shst:latest /bin/bash
> node@32da0c9ad310:/$ shst help
bash: /home/node/.npm-global/bin/shst: /usr/bin/env: bad interpreter: No such file or directory

This seems to be a path issue. I think I'm following the docker-node Best Practices. Any ideas how to make these play nicely together?

josiekre commented 5 years ago

Oddly, the -v seems to be part of the problem.

$ docker run -it --rm shst:latest shst help
SharedStreets, a 'digital commons' for the street

VERSION
  sharedstreets/0.12.0 linux-x64 node-v11.15.0

USAGE
  $ shst [COMMAND]

COMMANDS
  extract  extracts SharedStreets streets using polygon boundary and returns GeoJSON
           output of all intersecting features
  help     display help for shst
  match    matches point and line features to sharedstreets refs

Still looking...

josiekre commented 5 years ago

I think the Docker is working now once I point the volume to a directory that matches with the default user node, but there's a different issue. So I will file that separately.

docker run -it --rm -v ~/Downloads/:/usr/node/ shst:latest shst match /usr/node/line_2.in.geojson --out=/usr/node/match.geojson
  🌏  Loading geojson data...
       Matching using car routing rules on all streets
  ✨  Matching 1 lines...
TypeError: Cannot set property '_maxEntries' of undefined
    at RBush (~/.npm-global/lib/node_modules/sharedstreets/node_modules/rbush/rbush.js:65:22)
    at Object.geojsonRbush [as default] (~/.npm-global/lib/node_modules/sharedstreets/node_modules/geojson-rbush/index.js:22:16)
    at new TileIndex (~/.npm-global/lib/node_modules/sharedstreets/build/src/tile_index.js:66:57)
    at new Graph (~/.npm-global/lib/node_modules/sharedstreets/build/src/graph.js:300:30)
    at ~/.npm-global/lib/node_modules/sharedstreets/build/src/commands/match.js:355:23
    at Generator.next (<anonymous>)
    at ~/.npm-global/lib/node_modules/sharedstreets/build/src/commands/match.js:7:71
    at new Promise (<anonymous>)
    at __awaiter (~/.npm-global/lib/node_modules/sharedstreets/build/src/commands/match.js:3:12)
    at matchLines (~/.npm-global/lib/node_modules/sharedstreets/build/src/commands/match.js:277:12)

Is there interest in having this Docker in the repo and instructions in the Readme? I can create a pull request if so.

kpwebb commented 5 years ago

@josiekre creating a docker install is a great idea. We'll work on that for the next release. We're also looking into support for Node v8.

That said, we pushed a fix today for the Cannot set property '_maxEntries' of undefined problem you identified here and in #50. Another package that we depend on broke last week causing this error. We've replaced it as a dependency and the CLI should now work on Node v10+.

sharedstreets / sharedstreets-js

Specify node engine version #48