peerlibrary / meteor-computed-field

Reactively computed field for Meteor
https://atmospherejs.com/peerlibrary/computed-field
BSD 3-Clause "New" or "Revised" License
18 stars 6 forks source link

Revert to coffeescript 1.0.17. Version bump to 0.9.0. Closes #13 #14

Closed xet7 closed 6 years ago

xet7 commented 6 years ago

Revert to coffeescript 1.0.17. Version bump to 0.9.0. Closes #13

mitar commented 6 years ago

Isn't this the same as version 0.7.0?

xet7 commented 6 years ago

@mitar

Yes, this reverts to 0.7.0 and adds downgraded dependency coffeescript to 1.0.17 so that this package works on Meteor 1.6.1.

xet7 commented 6 years ago

@mitar

Without downgraded dependency to coffeescript 1.0.17 etc it is not possible to upgrade Meteor projects to Meteor 1.6.1.

mitar commented 6 years ago

Isn't version 0.7.0 of this package published with coffeescript 1.0.17 already?

xet7 commented 6 years ago

@mitar

No, version 0.7.0 does not specify coffeescript version, so it breaks upgrades.

mitar commented 6 years ago

No, version 0.7.0 does not specify coffeescript version, so it breaks upgrades.

It does by specifying api.versionsFrom.

xet7 commented 6 years ago

@mitar

Specifying api.versionsFrom did not work. Version 0.7.0 is broken. Only specifying coffeescript version 1.0.17 separately fixed this.

xet7 commented 6 years ago

@mitar

Without this fix, everyone needs to manually monkey patch like at https://github.com/peerlibrary/meteor-computed-field/issues/13#issuecomment-365400328

mitar commented 6 years ago

Why version 1.0.17 and not 1.2.4 of CoffeeScript?

xet7 commented 6 years ago

@mitar

I have not tried 1.2.4 yet. Just a moment I try.

xet7 commented 6 years ago

Yes 1.2.4 works.

xet7 commented 6 years ago

Hmm, seems to still have errors:

This project is already at Meteor 1.6.1, the latest release.
=> Errors while upgrading packages:           

While selecting package versions:
error: Conflict: Constraint coffeescript@1.2.4 is not satisfied by coffeescript 1.0.17.
Constraints on package "coffeescript":
* coffeescript@1.0.5 <- peerlibrary:blaze-components 0.15.1
* coffeescript@1.0.5 <- peerlibrary:base-component 0.16.0 <- peerlibrary:blaze-components 0.15.1
* coffeescript@1.0.5 <- peerlibrary:reactive-field 0.3.0 <- peerlibrary:base-component 0.16.0 <-
peerlibrary:blaze-components 0.15.1
* coffeescript@1.0.5 <- peerlibrary:reactive-field 0.3.0 <- peerlibrary:blaze-components 0.15.1
* coffeescript@1.2.4 <- peerlibrary:computed-field 0.9.0
* coffeescript@1.0.4 <- softwarerero:accounts-t9n 1.3.11 <- useraccounts:core 1.14.2
* coffeescript@1.0.3 <- 3stack:presence 1.1.2
* coffeescript@1.0.5 <- alethes:pages 1.8.6
* coffeescript@1.0.4 <- arillo:flow-router-helpers 0.5.2
* coffeescript@1.0.4 <- zimme:active-route 2.3.2 <- arillo:flow-router-helpers 0.5.2
* coffeescript@1.0.4 <- tap:i18n 1.8.2
* coffeescript@1.0.4 <- templates:tabs 2.3.0
xet7 commented 6 years ago

Only 1.0.17 works:

$ meteor update
This project is already at Meteor 1.6.1, the latest release.
Your top-level dependencies are at their latest compatible versions.

The following top-level dependencies were not updated to the very latest version available:
 * aldeed:collection2 2.10.0 (3.0.0 is available)
 * matteodem:easy-search 1.6.4 (2.0.0 is available)
 * ongoworks:speakingurl 1.1.0 (9.0.0 is available)
 * peerlibrary:blaze-components 0.15.1 (0.22.0 is available)

Newer versions of the following indirect dependencies are available:
 * aldeed:collection2-core 1.2.0 (2.1.2 is available)
 * aldeed:schema-deny 1.1.0 (3.0.0 is available)
 * aldeed:schema-index 1.1.1 (3.0.0 is available)
 * coffeescript 1.0.17 (2.2.1_1 is available) 
 * peerlibrary:reactive-field 0.3.0 (0.4.0 is available)
 * softwarerero:accounts-t9n 1.3.11 (2.2.1 is available)
These versions may not be compatible with your project.
To update one or more of these packages to their latest
compatible versions, pass their names to `meteor update`,
or just run `meteor update --all-packages`.
xet7 commented 6 years ago

With this pull request Wekan works with Meteor 1.6.1, Node 8.9.4 and 5.6.0, as you can see from screenshot below. Wekan is used by single private persons to companies that have thousands of users. We really would like to get rid of all of monkey patches and contribute them upstream, so we don't have to fork so much packages. I did yesterday write blog post about benefits of contributing features and bugfixes to upstream https://blog.wekan.team/2018/02/benefits-of-contributing-your-features-to-upstream-wekan/index.html , like contributing features and bugfixes to Wekan. peerlibrary is upstream library that Wekan uses.

wekan-meteor-1 8 1

xet7 commented 6 years ago

@mitar

Is there still something to fix in this pull request? What prevents merging and releasing this?

mitar commented 6 years ago

As you see from the list above:

* coffeescript@1.0.4 <- softwarerero:accounts-t9n 1.3.11 <- useraccounts:core 1.14.2
* coffeescript@1.0.3 <- 3stack:presence 1.1.2
* coffeescript@1.0.5 <- alethes:pages 1.8.6
* coffeescript@1.0.4 <- arillo:flow-router-helpers 0.5.2
* coffeescript@1.0.4 <- zimme:active-route 2.3.2 <- arillo:flow-router-helpers 0.5.2
* coffeescript@1.0.4 <- tap:i18n 1.8.2
* coffeescript@1.0.4 <- templates:tabs 2.3.0

These packages are forcing your whole app to be in 1.0 CoffeeScript. I would suggest you also send upstream patches to those packages to upgrade to at least 1.2. Then this 0.7.0 version will work.

xet7 commented 6 years ago

@mitar

Thanks! I'll send upstream patches.

mitar commented 6 years ago

Can you list what versions of those packages I listed above you have in your app before you upgrade to Meteor 1.6.1?

mitar commented 6 years ago

So what I do not understand is why having CoffeeScript 1.2 and 1.0 works in the same app before Meteor 1.6.1.

xet7 commented 6 years ago

@mitar

Here is current packages for 1.6.0.1: https://github.com/wekan/wekan/blob/devel/.meteor/versions

mitar commented 6 years ago

So what seems to happen is that after upgrade to Meteor 1.6.1 you can choose only between CoffeeScript 2, CoffeeScript 1.2 or CoffeeScript 1.0. Some packages are on 1.0 (listed above), this package 0.7.0 is on CoffeeScript 1.2 and 0.8.0 on CoffeeScript 2. Sadly, the CoffeeScript 1.0 version of this package is broken.

xet7 commented 6 years ago

@mitar

So could these all packages work with CoffeeScript 2 with some package upgrades?

mitar commented 6 years ago

Ehm, in versions you linked above, you have coffeescript@1.12.7_3. This is even newer than 1.2.4 for which this package 0.7.0 was released. So why you cannot keep coffeescript@1.12.7_3 when upgrading to Meteor 1.6.1?

xet7 commented 6 years ago

@mitar

Just a moment I try.

mitar commented 6 years ago

So could these all packages work with CoffeeScript 2 with some package upgrades?

That is a separate process all of packages using CoffeeScript in Meteor will eventually have to do. I have updated many of my own packages (including this one) to CoffeeScript 2 because some users are already using that.

But yes, that would also solve a problem you are having, if everyone just upgrade to CoffeeScript 2.

xet7 commented 6 years ago

@mitar

Ok, I'll all pull requests for upgrades to CoffeeScript 2, it's a better solution.

mahmost commented 6 years ago

I Agree that the CoffeeScript 2 upgrade is the better long term choice .. I also think that maintaining a 0.7.1 version with this change 1.0.17 is not bad .. just to keep things working with meteor 1.6.1 for now ..

And for the tests to pass .. I think adding package constraint to line 35 in package.js will do the trick

xet7 commented 6 years ago

@mahmost

Downgrading to old package is still monkey patching to probably broken versions, and that's already possible. I would like to upgrade all packages instead.

xet7 commented 6 years ago

@mitar

Can you release this as v0.7.1 ? I tried to upgrade all dependencies, but got lost in the process.

xet7 commented 6 years ago

@mitar

It seems that with these dependencies it's not possible to use coffeescript@1.12.7_3, it downgrades to 1.0.17 anyway.

xet7 commented 6 years ago

Sadly, the CoffeeScript 1.0 version of this package is broken.

Ok.