alexa-js / alexa-app

A framework for Alexa (Amazon Echo) apps using Node.js
https://www.youtube.com/watch?v=pzM4jv7k7Rg
MIT License
1.03k stars 213 forks source link

Added TypeScript declarations for the Alexa address objects #324

Closed CodeCutterUK closed 6 years ago

CodeCutterUK commented 6 years ago

Added TypeScript declarations for responses returned from the device-api

CodeCutterUK commented 6 years ago

Looks like builds for node 1 to 4 are failing, but I don't see how my commit would have caused this :)

kobim commented 6 years ago

The failure actually relates to npm v2, which tries to satisfies the peerDependency "typescript": ">=2.1.0 || >=2.1.0-dev || >=2.2.0-dev || >=2.3.0-dev || >=2.4.0-dev || >=2.5.0-dev || >=2.6.0-dev || >=2.7.0-dev || >=2.8.0-dev" from tslint, but dtslint is using "typescript": "next" which resolves to 2.9.0-dev (and npm versions below 3 fail if there's a peerDependency conflict).

I've opened palantir/tslint#3792 which should resolve this errors, but I think it's safe to ignore them for now as they relate to the linter, and not to the actual performance of the library.

@dblock what do you think?

dblock commented 6 years ago

Well I don't want to check in something that turns the build to broken. Can we resolve this by locking down things to an older version?

kobim commented 6 years ago

dtslint@0.1.2 passes the installation, but fails as it uses arrow functions (which are only supported since node v4.) The current version of dtslint supports node>=6, and if we understand that we only use the dtslint at the Node.js: next job of the CI, I think it's safe to remove it from our devDependencies (.travis.yml already installs it, we just need to make sure it installs the right version.)

I've submitted #329 with this fix attempt, and it seems to address the issue. If accepted and merged, contributors could sync-merge their PRs and we will be able to address their real commits instead of our build failures :)

CodeCutterUK commented 6 years ago

@kobim Got a clean build now, thanks for fixing the dtslint issue

dblock commented 6 years ago

We can merge this right? @lazerwalker? (go for it)