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

Updated dependencies for aurelia-binding (removed 1.x dependency) #897

Closed
wants to merge 1 commit into from

Conversation

bulldetektor
Copy link

To be able to install via JSPM (which doesn't support comparator sets for versioning)

Fixes #895.

Note; got 1 failing specs test when running karma start:

WARN [karma]: No captured browser, open http://localhost:9876/
INFO [karma]: Karma v1.7.1 server started at http://0.0.0.0:9876/
INFO [launcher]: Launching browser Chrome with unlimited concurrency
INFO [launcher]: Starting browser Chrome
INFO [Chrome 67.0.3396 (Windows 10.0.0)]: Connected on socket 7yMa3-xOliKE8IMbAAAA with id 94643261
Chrome 67.0.3396 (Windows 10.0.0) aurelia setRoot() should accept view model class as root FAILED
Expected Function to be Function.
at eval (test/aurelia.spec.js:264:52)
at tryCatchReject (jspm_packages/system-polyfills.src.js:1188:30)
at runContinuation1 (jspm_packages/system-polyfills.src.js:1147:4)
at Fulfilled.when (jspm_packages/system-polyfills.src.js:935:4)
Chrome 67.0.3396 (Windows 10.0.0): Executed 35 of 35 (1 FAILED) (0.146 secs / 0.09 secs)

... but I'm not sure if this is related to the updated dependency versions

…be able to install via JSPM (which doesn't support comparator sets for versioning)

Fixes aurelia#895.
@EisenbergEffect
Copy link
Contributor

@bulldetektor Is it possible to leave the NPM versions as is and just update the JSPM versions?

@bulldetektor
Copy link
Author

bulldetektor commented Jun 28, 2018

Sure, but what will be the consequences of that? Isn't it a possibility that you'll end up with 1.3 as npm package and 2.0 in jspm? In practice I guess that would mean that you'll get the TypeScript definitions (d.ts) in VS Code for the 1.3, but in runtime (bundled) you'll be running the 2.0. So yes; I guess it could work, but I'd like to hear the reasoning behind stil having a dependency on aurelia-binding 1.x when 2.x is out.

@EisenbergEffect
Copy link
Contributor

@bigopon @StrahilKazlachev @fkleuver What were the reasons that we added the || in the semver range for binding? Why couldn't we just bump it to 2.0?

@arabsight
Copy link

also, webpack is bundling two versions of aurelia-binding:

├─ aurelia-binding@1.7.1
└─ aurelia-templating-binding@1.4.2
   └─ aurelia-binding@2.1.1

@doktordirk
Copy link

doktordirk commented Jun 28, 2018

@arabsight that probably only happens when you specifically install aurelia-binding ^1.7.0 aka it is in your package.json? the only one you need in your package.json is aurelia-bootstrapper (and plugins eg dialog, i18n...), but no core modules (other than aurelia-polyfills, which one needs it seems). then that shouldn't happen afaik

@arabsight
Copy link

@doktordirk nope, this is a default aurelia-cli generated project.

  • au new demo (accept all defaults)
  • yarn list aurelia-binding

@arabsight
Copy link

aurelia-templating-binding is the only package pulling aurelia-binding@2.1.1
it has this in the dependencies key:

"aurelia-binding": "^1.3.0 || ^2.0.0",

where all other packages that depend on aurelia-binding has:

"aurelia-binding": "^1.0.0 || ^2.0.0",

@fkleuver
Copy link
Member

fkleuver commented Jun 30, 2018

The reason for || is to retain the option of installing 1.x (backwards compatibility). If for whatever reason someone needs 1.x instead of 2.x and they install that explicitly with webpack, they'll get the same problems as they got before this change. There will be 2 versions installed.

Other than that, I simply kept the version on the left-hand side as it was.

@EisenbergEffect
Copy link
Contributor

I think we're going to need to push everything forward to the latest version. The || won't work for jspm but we can't move that to something different than npm since that will cause tooling to break. They need to stay in sync. I was going to propose to move everything forward to 2.x only unless someone has an alternate plan that will avoid all issues.

@doktordirk
Copy link

peerDependencies might have helped, but jspm 0.16 doesn't have them and jspm 0.17 is still beta after 2 years. just be reminded that the update needs to be on the non rc versions as the rc/beta versions are out of the normal semver range and without special effort (as it should be), the last non rc/beta versions will be installed (for npm that is, but i guess/hope it's similar for jspm)

@zewa666
Copy link
Member

zewa666 commented Jun 30, 2018

Have a similar situation over at the i18n plugin. I dont really see the need for backwards-compat 1.x. nevertheless i'll wait for this one and update i18n accordingly

@fkleuver
Copy link
Member

fkleuver commented Jun 30, 2018

I don't see a direct need for backwards compat either, but there will be problems regardless. This is a serious issue with jspm as far as I'm concerned. Lack of support for || is really bad.. that's been around for a while. See jspm/jspm-cli#2406

@bulldetektor
Copy link
Author

@EisenbergEffect Guess you can close this now, as it seems that you've aligned the dependencies across all repos now :)

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.

6 participants