Skip to content
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

Specify node engine version #48

Open
morganherlocker opened this issue Apr 22, 2019 · 4 comments
Open

Specify node engine version #48

morganherlocker opened this issue Apr 22, 2019 · 4 comments
Labels

Comments

@morganherlocker
Copy link
Contributor

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
Copy link

josiekre commented May 17, 2019

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
Copy link

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
Copy link

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
Copy link
Member

kpwebb commented May 21, 2019

@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+.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants