Meteor-Community-Packages / meteor-collection2

A Meteor package that extends Mongo.Collection to provide support for specifying a schema and then validating against that schema when inserting and updating.
https://packosphere.com/aldeed/collection2
MIT License
1.02k stars 108 forks source link

Meteor 3.0 migration #444

Closed harryadel closed 7 months ago

harryadel commented 8 months ago

A continuation of @klablink work

You can test the changes locally by updating the package version to 4.0.0-beta.7

harryadel commented 8 months ago

@StorytellerCZ @jankapunkt @klablink The test app got updated to 3.0-alpha.19 and the tests are passing. The meteortesting:mocha in use now is 3.0.3-alpha300.11 thanks to @Grubba27. Good call on you to mention your work in the excel file. I'd not have known otherwise.

jankapunkt commented 8 months ago

Perfect, a beta would be awesome

harryadel commented 8 months ago

coming up...

harryadel commented 8 months ago

A new version 4.0.0-beta.4 is published where the helper code made for setting sync or async DB calls got reworked.

cc @StorytellerCZ @jankapunkt @klablink

alisnic commented 8 months ago

A new version 4.0.0-beta.4 is published

2400 unit tests and 821 e2e tests pass in our product with this release 🎉 The only thing we needed to do is to remove schema-index package

harryadel commented 8 months ago

Ah finally someone reported in :tada:, thank you @alisnic.

Did you run into any problems? Did you have to go through any extra steps to migrate?

I was thinking of releasing a new beta where we're no longer reliant on the NPM version of simple-schema and instead use @jankapunkt fork. What do you think?

alisnic commented 8 months ago

Did you run into any problems?

Nope all, good. I had to remove schema-index since it's not compatible with this version. But we're not really using this package so it wasn't a problem

I was thinking of releasing a new beta where we're no longer reliant

Sounds good, I can make that run in our CI as well and report if you want

harryadel commented 8 months ago

Sounds good, I can make that run in our CI as well and report if you want

Thank you, brother. Ok, I'll push a new beta and let you know.

harryadel commented 8 months ago

@alisnic v4.0.0-beta.5 is ready to go!

alisnic commented 8 months ago

@harryadel all good, tests pass. We also have simpl-schema explicitly defined in package.json (v1.12.0). I didn't remove it and convert to import aldeed/simple-schema because we are using it outside Meteor specific done. Nevertheless, everything works fine

harryadel commented 8 months ago

That's wonderful to know. Thank you @alisnic

alisnic commented 8 months ago

@harryadel no biggie, glad I could help! When can we expect a full release?

harryadel commented 8 months ago

I don't have a date really, it's more about reaching a stable status where we're sure there're no bugs, at least to the best of our abilities. After all, nobody reported back but you.

I'd like to add static & dynamic loading like autoform since we're doing a major release and maybe include https://github.com/longshotlabs/meteor-schema-index and https://github.com/longshotlabs/meteor-schema-deny to be built in features. Most would agree with the dynamic/static loading but I need other opinions on the other two. Should we keep collection2 lean and simple and leave out the rest? I don't really know.

alisnic commented 8 months ago

@harryadel I understand your desire to create a comprehensive release, but I think it would be better to release often and in small chunks. There is already a huge area of changes with async migrations and mixing up other packages will create more potential room for risk.

From my perspective as a user I don't mind potential bugs, worst case we will just revert to old version and wait for a new release with fix (best case we will submit a patch with a fix). What I'm saying is that at least I have the choice to upgrade or not, right now the only option we have is to just wait.

I think it's important to get the package ecosystem to be ready before Meteor 3 migration, as users can prepare for the actual migration in advance, testing and making sure nothing breaks. This leaves a lot less things that change during the migration itself, reducing room for errors and bugs.

jankapunkt commented 8 months ago

We can actually do alpha releases with packages, too. Any release with alpha or beta suffix (semver style) will not be considered update candidate when users run meteor update. It's therefore safe to release alpha,/beta packages

harryadel commented 8 months ago

@alisnic I agree with what you've just said but given this is a major release I wanted to add as many breaking changes as possible instead of doing separate releases which would be tedious. So, if this beta 4.0.0-beta.6 which includes dynamic/static loading is ok. I'll do a full release afterwards immediately.

StorytellerCZ commented 8 months ago

Things are looking good, but will have to take another look once I recover from covid and have the opportunity to test things out.

xet7 commented 8 months ago

@harryadel

WeKan https://wekan.github.io currently uses Meteor 2.14. When I run meteor update, it breaks.

Info how to build wekan is at https://github.com/wekan/wekan/wiki/Emoji

mantic@m:~/repos/wekan$ cat .meteor/release
METEOR@2.14
mantic@m:~/repos/wekan$ meteor update
This project is already at Meteor 2.14, the latest release.

Changes to your project's package version selections from updating package versions:

aldeed:simple-schema     upgraded from 1.5.4 to 1.13.1
blaze                    upgraded from 2.7.1 to 2.8.0
blaze-tools              upgraded from 1.1.3 to 1.1.4
caching-html-compiler    upgraded from 1.2.1 to 1.2.2
html-tools               upgraded from 1.1.3 to 1.1.4
htmljs                   upgraded from 1.1.1 to 1.2.0
matb33:collection-hooks  upgraded from 1.3.0 to 1.3.1
mdg:validation-error     removed from your project
momentjs:moment          upgraded from 2.29.3 to 2.30.1
observe-sequence         upgraded from 1.0.21 to 1.0.22
spacebars                upgraded from 1.4.1 to 1.5.0
spacebars-compiler       upgraded from 1.3.1 to 1.3.2
templating-compiler      upgraded from 1.4.1 to 1.4.2
templating-tools         upgraded from 1.2.2 to 1.2.3
zodern:types             upgraded from 1.0.10 to 1.0.11

The following top-level dependencies were not updated to the very latest version available:
 * aldeed:collection2 2.10.0 (3.5.0 is available)
 * lmieulet:meteor-coverage 1.1.4 (4.1.0 is available)
 * ongoworks:speakingurl 1.1.0 (9.0.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)
 * meteortesting:browser-tests 1.4.2 (1.5.3 is available)
 * meteortesting:mocha-core 8.0.1 (8.1.2 is available)
 * raix:eventemitter 0.1.3 (1.0.0 is available)
 * templating 1.4.1 (1.4.3 is available)      
 * templating-runtime 1.5.0 (1.6.4 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`.
If the packages do not upgrade after this, this could mean
that there is a newer version of Meteor which the package
requires, but it not yet recommended or that some package
dependencies are not up to date and don't allow you to get
the latest package version.

~/repos/wekan$ ./rebuild-wekan.sh 
Recommended for development: Newest Ubuntu or Debian amd64, directly to SSD disk or dual boot, not VM. Works fast.
Note1: If you use other locale than en_US.UTF-8 , you need to additionally install en_US.UTF-8
       with 'sudo dpkg-reconfigure locales' , so that MongoDB works correctly.
       You can still use any other locale as your main locale.
Note2: Console output is also logged to ../wekan-log.txt

 1) Install Wekan dependencies
 2) Build Wekan
 3) Run Meteor for dev on http://localhost:4000
 4) Run Meteor for dev on http://localhost:4000 with trace warnings, and warnings using old Meteor API that will not exist in Meteor 3.0
 5) Run Meteor for dev on http://localhost:4000 with bundle visualizer
 6) Run Meteor for dev on http://CURRENT-IP-ADDRESS:4000
 7) Run Meteor for dev on http://CURRENT-IP-ADDRESS:4000 with MONGO_URL=mongodb://127.0.0.1:27019/wekan
 8) Run Meteor for dev on http://CUSTOM-IP-ADDRESS:PORT
 9) Save Meteor dependency chain to ../meteor-deps.txt
10) Quit
Please enter your choice: 3
[[[[[ ~/repos/wekan ]]]]]

=> Started proxy.
=> Started MongoDB.
I20231230-21:21:44.197(2)? > Starting board-background-color migration.
I20231230-21:21:44.253(2)? > Finishing board-background-color migration.
...
I20231230-21:21:45.828(2) (percolate_synced-cron.js:87) SyncedCron: Scheduled "notification_cleanup" next run @Sat Dec 30 2023 21:21:45 GMT+0200 (Itä-Euroopan normaaliaika)
=> Started your app.

=> App running at: http://localhost:4000
W20231230-21:30:36.596(2)? (STDERR) /home/mantic/.meteor/packages/meteor-tool/.2.14.0.jneyca.onft++os.linux.x86_64+web.browser+web.browser.legacy+web.cordova/mt-os.linux.x86_64/dev_bundle/server-lib/node_modules/fibers/future.js:280
W20231230-21:30:36.599(2)? (STDERR)                         throw(ex);
W20231230-21:30:36.600(2)? (STDERR)                         ^
W20231230-21:30:36.600(2)? (STDERR) 
W20231230-21:30:36.600(2)? (STDERR) ReferenceError: SimpleSchema is not defined
W20231230-21:30:36.600(2)? (STDERR)     at packages/aldeed_collection2-core.js:37:35
W20231230-21:30:36.601(2)? (STDERR)     at packages/aldeed_collection2-core.js:698:4
W20231230-21:30:36.601(2)? (STDERR)     at packages/aldeed_collection2-core.js:706:3
W20231230-21:30:36.601(2)? (STDERR)     at /home/mantic/repos/wekan/.meteor/local/build/programs/server/boot.js:385:38
W20231230-21:30:36.601(2)? (STDERR)     at Array.forEach (<anonymous>)
W20231230-21:30:36.602(2)? (STDERR)     at /home/mantic/repos/wekan/.meteor/local/build/programs/server/boot.js:210:21
W20231230-21:30:36.602(2)? (STDERR)     at /home/mantic/repos/wekan/.meteor/local/build/programs/server/boot.js:439:7
W20231230-21:30:36.602(2)? (STDERR)     at Function.run (/home/mantic/repos/wekan/.meteor/local/build/programs/server/profile.js:256:14)
W20231230-21:30:36.602(2)? (STDERR)     at /home/mantic/repos/wekan/.meteor/local/build/programs/server/boot.js:438:13
=> Exited with code: 1
=> Your application is crashing. Waiting for file change.
harryadel commented 8 months ago

@xet7 Are you using https://github.com/Meteor-Community-Packages/meteor-simple-schema ?

xet7 commented 8 months ago

@harryadel

Currently I'm using these. meteor update breaks it.

https://github.com/wekan/wekan/blob/main/.meteor/versions#L4-L8

xet7 commented 8 months ago

@harryadel

I have also tried this Meteor 3 PR, forking it to wekan/packages, but it does not work with Meteor 2.14.

xet7 commented 8 months ago

@harryadel

After doing meteor update, I also tried with npm remove simpl-schema, but I still get error SimpleSchema is not defined.

harryadel commented 8 months ago

Thanks for reporting it, @xet7. I'll look it into it.

StorytellerCZ commented 8 months ago

@xet7 at the first look, the versions are a bit of a mess. You are using v2 of collection hook, while schema-deny and schema-index are at v1, you will first need to manually update to v3 and use the npm version of simple-schema. Once that is aligned the error should go away. As far as I can see it has nothing to do with this PR and the v4 betas.

Update: @xet7 I have a PR for you soon.

harryadel commented 7 months ago

4.0.0-beta.7 solves a issue when using collection2 with quave:synced-cron. quave:synced-cron checks for the error code if an entry fails and returns/fails silently if it's a duplicate entry. This works fine when synced-cron is used on its own but when paired with collection2 which modifies Meteor's collection internals and wraps up errors, this becomes a problem. So, now collection2 passes the error code when throwing out an error.

https://github.com/quavedev/meteor-synced-cron/blob/7212459895924a866f8e45e98026164d5f43fd15/synced-cron-server.js#L221

I've a fork of quave:synced-cron that validates this problem where I overhauled the tests and the CI fails as aldeed:collection2@4.0.0-beta.6 is in use. But I'm not able to create PR to merge it, why tho? :thinking: cc @filipenevola

jankapunkt commented 7 months ago

@harryadel I will update tests on lai:collection-extensions so they're using beta.7 and I'll let you know asap if things are working

jankapunkt commented 7 months ago

After updating to beta 7 I can't run tests anymore:

=> Started proxy.                             
/home/jankapunkt/.meteor/packages/meteor-tool/.3.0.0-beta.0.ag3n3e.4pp4++os.linux.x86_64+web.browser+web.browser.legacy+web.cordova/mt-os.linux.x86_64/tools/runners/run-all.js:170
          Console.log("Error starting Mongo (".concat(left, " left): ").concat(error.message));
                  ^

[TypeError: Console.log is not a function
  at /home/jankapunkt/.meteor/packages/meteor-tool/.3.0.0-beta.0.ag3n3e.4pp4++os.linux.x86_64+web.browser+web.browser.legacy+web.cordova/mt-os.linux.x86_64/tools/runners/run-all.js:170:19
  at runNextTicks (node:internal/process/task_queues:60:5)
  at listOnTimeout (node:internal/timers:540:9)
  at processTimers (node:internal/timers:514:7)
]
harryadel commented 7 months ago

Will you please push your latest changes here so I can test things out?

filipenevola commented 7 months ago

But I'm not able to create PR to merge it

What is happening? We don't have any unique settings in this repo, not on purpose, at least.

Feel free to reach me in pvt as well if you need help sending the PR. Contributions are always welcome.

DanielDornhardt commented 7 months ago
Console.log("Error starting Mongo (".concat(left, " left): ").concat(error.message));

Sorry if this is super besides the point, I just want to mention that I remember console being spelled with a lowercase "c" instead of an uppercase C. Although I guess the real question is probably another, I just wanted to add a drive-by comment...

minhna commented 7 months ago

I tried and it works. The app works. I have a problem when run meteor test --driver-package meteortesting:mocha Error: TypeError: Settings.attachSchema is not a function Settings is one of my collection. Then I need to add this import to the .test file:

import "meteor/aldeed:collection2/static";
harryadel commented 7 months ago

@minhna There're two breaking changes first the replacement of NPM simpl-schema where we now use @jankapunkt fork and static/dynamic loading. But otherwise it's perfectly compatible.

minhna commented 7 months ago

Do we have another way to load collection2 in test mode? I added that import to one of my test files (which reported error) and it works with other files but I feel it's not the right way doing it.

harryadel commented 7 months ago

You can also dynamically load it

https://github.com/Meteor-Community-Packages/meteor-collection2/pull/444/commits/841a7b3b8e94d792448ab24b3a0c12a898101334

harryadel commented 7 months ago

4.0.0 is released :tada: , thank you guys!