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

Removed package.json engine restrictions #195

Merged
merged 1 commit into from
Oct 12, 2016
Merged

Conversation

ocombe
Copy link
Contributor

@ocombe ocombe commented Oct 12, 2016

What kind of change does this PR introduce?

  • Refactoring (no functional changes, no api changes)

What is the current behavior?
The lib restricts the engine to node ~6.0.0 for no reason (this lib doesn't use any binary). This is a problem when using Yarn because it is more strict than npm and it will respect the engine restrictions which will fail the installation of all the npm modules with the following error message:
error angular2-data-table@0.9.3: The engine "node" is incompatible with this module. Expected version "~6.0.0".

What is the new behavior?
Removed the restriction

Does this PR introduce a breaking change?

  • Yes
  • No

@amcdnl
Copy link
Contributor

amcdnl commented Oct 12, 2016

I added the engine restrictions to prevent people from downloading it with Node 4.x and it having build errors.

@ocombe
Copy link
Contributor Author

ocombe commented Oct 12, 2016

you only have build errors if you clone it from github and try to run it, not if you install it from npm right ?

@amcdnl
Copy link
Contributor

amcdnl commented Oct 12, 2016

Correct.

@ocombe
Copy link
Contributor Author

ocombe commented Oct 12, 2016

Then the engine restriction in the package.json doesn't help, because it only works when you npm install it from npm :)

@amcdnl amcdnl merged commit 5024a1a into swimlane:master Oct 12, 2016
@amcdnl
Copy link
Contributor

amcdnl commented Oct 12, 2016

Thanks! :)

@niklas-dahl
Copy link
Contributor

I'm using node v6.3.1 and I still get this error using yarn, is this intended?

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

Successfully merging this pull request may close these issues.

4 participants