-
Notifications
You must be signed in to change notification settings - Fork 33
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
fixes package for Meteor 2.3 and Meteor 3.0 #88
base: master
Are you sure you want to change the base?
Conversation
package.js
Outdated
documentation: "README.md", | ||
git: "https://github.com/dburles/meteor-collection-helpers.git", | ||
}); | ||
|
||
Package.onUse(function(api) { | ||
api.versionsFrom('1.4.2'); | ||
api.versionsFrom(['1.12.1', '2.3', '3.0']); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @drone1 just wondering why the minimum version bump here is required?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, actually it looks like you're referring to the package minimum version bump but you replied on this line about minimum Meteor versions?
If so, I bumped the package version since there is an update? Sorry I'm not understanding the question. Thank you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the specific trouble you were having running the current version of the package on Meteor 3? It works okay for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I meant here is that you have dropped support for Meteor versions between 1.4.2 and 1.12.1 and I was wondering if there was any specific reason for that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the specific trouble you were having running the current version of the package on Meteor 3? It works okay for me.
Helpers were not present on a client collection on document retrieved with findOne()
. If I used collection._transform()
the helpers appeared again as expected.
Once I changed the api.versionFrom
line everything worked again from what I recall, but let me confirm and get back here with details.
What I meant here is that you have dropped support for Meteor versions between 1.4.2 and 1.12.1 and I was wondering if there was any specific reason for that?
This was totally copy-pasta :( Will fix. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't reproduce the problem you mention, maybe something else is going on?
Hey! Thanks for the reply.
To be honest I copied this from aldeed:collections2, which works with
modern versions of Meteor, and it fixed it – I’m not sure what it means to
have three minimum versions.
…On Sun 29. Sep 2024 at 22:57, David Burles ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In package.js
<#88 (comment)>
:
> documentation: "README.md",
git: "https://github.com/dburles/meteor-collection-helpers.git",
});
Package.onUse(function(api) {
- api.versionsFrom('1.4.2');
+ api.versionsFrom(['1.12.1', '2.3', '3.0']);
Hey @drone1 <https://github.com/drone1> just wondering why the minimum
version bump here is required?
—
Reply to this email directly, view it on GitHub
<#88 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABQIEPAWY6A7QI5CC52CZCLZZBSUDAVCNFSM6AAAAABPA7LCPSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGMZVHE4TOMBRGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
In a new meteor 3.0.3 Blaze project, if I run:
it installs an outdated version, v1.0.0. If I remove that package and attempt to install v1.1.0, the most recent version, with
I get the following errors:
Is that expected? I get the same errors if I attempt to |
…om()` version (in dburles:[email protected]) of 1.4.2 with 1.12.1. this change corrects this (back to 1.4.2) but also retains the fix for meteor 3, which passes 3 versions rather than just a single version, and includes 2.3 and 3.0. Tests are passing for Meteor 2.3 and 3.0, using commands: ``` meteor test-packages ./packages/collection-helpers --release 2.3 meteor test-packages ./packages/collection-helpers --release 3.0 ``` Meteor 1.4.2 is spewing some errors about a failure to "get local issuer certificate" (macOS Sonoma 14.5), but I wanted to push this commit in case author @dburles wants to try and test.
In a Meteor 3.0.3 project, if I try to add my local package (with the corrected
However, if I change the line to Actually, I can't even get into a state where I can test the original problem I was having, because I cannot install versions 1.0.0 or 1.1.0 to my project, even with But yes, tests pass for Meteor 2.3 and 3.0. However I can't get up and running with Meteor 1.4.2:
I tried running with Thanks. |
Hey thanks for taking the time to test on earlier versions of Meteor. I'm happy with the version changes. Did you want to investigate async helpers? |
Ok great!
Yeah, I can try and take a look at some point and submit another PR. I’ve
needed async helpers myself now with the recent meteor updates…
…On Tue 15. Oct 2024 at 23:58, David Burles ***@***.***> wrote:
Hey thanks for taking the time to test on earlier versions of Meteor. I'm
happy with the version changes. Did you want to investigate async helpers?
—
Reply to this email directly, view it on GitHub
<#88 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABQIEPHVFTK3QYMW7ZEJFZTZ3WFYVAVCNFSM6AAAAABPA7LCPSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMJVGIZTONJQGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Great. I don't imagine it would require any changes to the source to support them. It's more a matter of including some tests for Meteor 3. |
Good point. Let me see what I can do.
…On Wed, Oct 16, 2024 at 1:06 AM David Burles ***@***.***> wrote:
Great. I don't imagine it would require any changes to the source to
support them. It's more a matter of including some tests for Meteor 3.
—
Reply to this email directly, view it on GitHub
<#88 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABQIEPG4CIRKSDIAF5VCMKDZ3WNYBAVCNFSM6AAAAABPA7LCPSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMJVGMYTOMZTGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Any news? |
Haven’t played with this yet. Are you waiting for these changes to accept
the PR? For some reason I was under the impression we were talking about a
secondary PR.
…On Mon 28. Oct 2024 at 23:30, Mattia Corbelli ***@***.***> wrote:
Any news?
—
Reply to this email directly, view it on GitHub
<#88 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABQIEPHZPGWVUAEVQK5ZLA3Z52UHJAVCNFSM6AAAAABPA7LCPSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINBSG44TIOBVG4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
I don't have any idea on how Meteor 3 works with non async queries. Where you have commented out a test with a todo it would be good to add some async helpers. |
Ah sure, let me take a look…
…On Wed 30. Oct 2024 at 08:49, David Burles ***@***.***> wrote:
I don't have any idea on how Meteor 3 works with non async queries. Where
you have commented out a test with a todo it would be good to add some
async helpers.
—
Reply to this email directly, view it on GitHub
<#88 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABQIEPBXTXZR5A72CLG3U63Z6CFPRAVCNFSM6AAAAABPA7LCPSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINBWGA4DSNJSGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Can't seem to get tests running on the server, despite a variety of attempts. Do you have any experience with this? I ended up posting on the Meteor forums but haven't heard anything yet. If you've got a tip, please let me know! |
passes all tests for meteor 1.12.1, 2.3, 3.0.3