Upgrade to v2 of coffee #172

brucejo75 · 2018-02-16T15:46:53Z

Make meteor-file-collection work with V2 of Coffescript (specifically 2.0.3.3).

Changes to `package.js`

I make the project dependent upon [email protected]_3.
I reset the version of vsivsi:meteor-file-collection to 2.0.0. Because of the dependency on [email protected]_3.

I believe all changes are backward compatible with v1 of CoffeeScript. But I did not retest them.

3 areas needed to be addressed:

`super` and `this`

Needed to refactor the FileCollection class constructor for both client and server.

`FileCollection.super` does not exist

Simply set FileCollection.__super__ to Mongo.Collection.prototype

String interpolations behave differently

In v1 a string interpolation for an ObjectID would resolve to the string portion of the object e.g. the _str property. This is no longer the case with v2.

…ded classes, __super__ issue, Interpolated strings issue.

vsivsi · 2018-02-16T21:26:09Z

Thanks for getting this started @brucejo75! Do you intend to keep going with the three areas listed above? I'll be happy to merge and push a release once everything looks ready.

brucejo75 · 2018-02-16T23:01:55Z

Thanks @vsiverson!

Yeah, I did those fixes in the pr and it all works now for my project in v1.6.1.

I am trying to find the errors in the tests. I fixed one.

Working on the resumable.js issues now. I suspect I have one error that is messing up all of the tests....

Slow going, I am Coffeescript illiterate and resumable.js just looks like a big hunk of stuff...

BTW: what do you think about changing the version to 2.0.0? I thought it was the only way to go because CS 1 & 2 are incompatible.

brucejo75 · 2018-02-16T23:21:08Z

Hey @vsiverson, can you point me to where the Basic resumable.js ... tests are poking file-collection?

I am just dropping on console.log statements in functions that look like they are the ones.

Or better yet, how can I run a debugger with spacejam? meteor test-packages starts up but the web interface (localhost:3000) is not connecting.

vsivsi · 2018-02-17T00:23:18Z

@brucejo75 Hey, thanks for working on this today. Unfortunately I'm busy for the rest of the day, I'll try to take a look at what you've done over the weekend and try to answer your questions above. I'm pretty rusty on this code myself at this point. Reminder to self, I should probably look into updating resumable.js as well in this release.

Resumable is definitely a messy codebase, and there is a fork that seems to be better maintained (name is escaping me), but I'm probably not going to put in the work to manage a probably breaking change like that.

brucejo75 · 2018-03-02T20:13:42Z

Hi @vsiverson, can you point me to where to look? I will be happy to run this down. Thanks!

vsivsi · 2018-03-06T03:09:56Z

Okay, I took a quick look. The tests only seem to be failing on the server-side which is curious, the same tests run from the client are all working. It's not immediately clear to me what would cause that...

Note that resumable.js is not actually being used in any of these failing tests. These are tests of the backend API that resumable.js uses, but the tests themselves are simple and mock the library. So there's no need to be digging around in resumable.js itself.

The resumable.js mock is:
https://github.com/vsivsi/meteor-file-collection/blob/master/test/file_collection_tests.coffee#L465-L524

The failing tests start here (again, failing on the server-side only):
https://github.com/vsivsi/meteor-file-collection/blob/master/test/file_collection_tests.coffee#L534

Not sure if that helps you out much or not. I really haven't had time to come up to speed on the differences in CoffeeScript2 because my motivation to do so is low, given that I do not currently plan to use Coffeescript (1 or 2) in any future projects.

vsivsi · 2018-03-06T03:21:41Z

I just tried running off your branch and I am seeing the same errors on the server-side when I run meteor test-packages (the web-UI is running for me). What is strange is the vanilla HTTP API endpoints are working fine, and no errors are appearing in the console, so something is subtly wrong with the tests, but only when run on the server.... Hmmm

edemaine · 2018-03-06T13:55:26Z

Just looking quickly: https://github.com/vsivsi/meteor-file-collection/blob/master/test/file_collection_tests.coffee#L465-L524 has some string interpolations of _id that probably need the ._str suffix as in the rest of this PR.

brucejo75 · 2018-03-06T20:14:13Z

Thanks @vsiverson for your response. Thanks @edemaine for identifying the issues!

@vsiverson, all tests now pass. Ready to roll!

vsivsi · 2018-03-06T20:33:10Z

Great news!

Okay, so I'm super annoyed that for any version after Meteor 1.6, the localhost:3000 interface for meteor test-packages doesn't reliably run the tests anymore. I had it working last night, but now I can't get it to work again.

Some other TODOs before release...

Make the damn meteor test-packages web interface reliable again.
Update all of the Atmosphere packages to latest versions
Update some npm packages, as makes sense
Update included version of resumable.js
Make sure everything still works with above updates
Test that the file-collection sample app works correctly with Meteor 1.6 and the updated file-collection.
Coordinate release of updated file-collection with updated job-collection, and ensure that both work correctly with the combined sample app

Also, @brucejo75 mentioned a major version bump to 2.0... Something like that is probably necessary. But here's an idea to ponder....

Some time ago I created a new Github Org called "Darkmattergy" (like Dark Matter + Energy, in keeping with the Meteor cosmic theme). I'm considering transferring file-collection and job-collection to that org on github, and setting up matching orgs on Atmosphere/NPM so that these projects and their associated bits can live on without my direct involvement. The "vsivsi" versions will deprecate with their Meteor 1.5.x support intact, and for Meteor 1.6+, everything will switch over to the new Org.

The job-collection package is a lot more popular than this one, and a similar effort to port it over with minimal involvement from me has happened there, so these are both pretty close to being ready to release.

Anyway, this seems like the best moment to make such a move if it is ever going to happen. We have breaking changes in both these packages and the Meteor + Coffeescript platform. There's a clear boundary here between what can be deprecated and what can be supported moving forward etc.

What do you guys think? Are you willing to be founding members of this new org (with the privileges and responsibilities that may entail?)

vsivsi · 2018-03-06T20:37:25Z

BTW, here are all of the latest versions of the dependent packages...

Package.onUse(function(api) {
  api.use('[email protected]_1', ['server','client']);
  api.use('[email protected]', 'server');
  api.use('[email protected]', ['server', 'client']);
  api.use('[email protected]', 'server');
  api.use('[email protected]', ['server', 'client']);
  api.addFiles('resumable/resumable.js', 'client');
  api.addFiles('src/gridFS.coffee', ['server','client']);
  api.addFiles('src/server_shared.coffee', 'server');
  api.addFiles('src/gridFS_server.coffee', 'server');
  api.addFiles('src/resumable_server.coffee', 'server');
  api.addFiles('src/http_access_server.coffee', 'server');
  api.addFiles('src/resumable_client.coffee', 'client');
  api.addFiles('src/gridFS_client.coffee', 'client');
  api.export('FileCollection');
});

Package.onTest(function (api) {
  api.use('vsivsi:file-collection@' + currentVersion, ['server', 'client']);
  api.use('[email protected]_1', ['server', 'client']);
  api.use('[email protected]', ['server', 'client']);
  api.use('[email protected]', ['server','client']);
  api.use('[email protected]', ['server','client']);
  api.use('[email protected]',['server','client']);
  api.use('[email protected]', ['server', 'client']);
  api.use('[email protected]', ['server', 'client']);
  api.use('[email protected]', 'client');
  api.addFiles('test/file_collection_tests.coffee', ['server', 'client']);
});

vsivsi · 2018-03-07T00:10:43Z

It's been so long, I somehow forgot that when running tests in a package directory you need to specify the CWD to the command: meteor test-packages ./. Doh.

brucejo75 · 2018-03-07T13:38:09Z

To be honest, my plan is to switch over to using fetch and formData or something like it. Along with something like node-fetch on the server.

I adopted File-Collection when I was first starting to use Meteor (prior to v1.3 and npm support).

For my application File-Collection is really overkill for what I am looking for and need. Plus I want to get my file data out of Mongo.

My goal with this PR was simply to get it running with Meteor 1.6.1.

Also, I know nothing about coffeescript. I do not understand the syntax and I think the whole concept is rife with issues (like adding additional dependency complexity that necessitated this PR).

I think your plan is a good one and the right thing to do. And I would be happy to help a little bit to get over the hump. But for my purposes it simply an exercise in doing the simplest thing for my app (make it work with Meteor v1.6.1 and not rewrite for fetch and formData yet.

edemaine · 2018-03-07T13:50:48Z

Sad to see people leaving both CoffeeScript and Meteor! I am still an active user of both, and of this package.

I'm willing to help (some) with this package (e.g. I still want to add proper mobile support, just haven't gotten to it yet), but less so with the entire organization.

vsivsi · 2018-03-07T17:48:01Z

I haven’t stopped liking CoffeeScript, it was a great tool for its time. But in the ES6+ world, it’s just less necessary, and with the rise of more complex JS build toolchains it just adds one more source of potential issues and maintenance (this PR being exemplary!) Plus TypeScript has a lot of merits for larger projects, but those merits are incompatible with the remaining benefits of CoffeeScript.

I’m also not down on Meteor per se, my work just no longer requires it. I don’t think it has in any way out-lived it’s usefulness. If anything, I think it has only recently stabilized to the point where I’d feel comfortable building something “real” with it. I do fear what happens if/when the MDG transitions to the next thing though. Meteor has a lot of moving parts, and that complexity will be difficult to maintain on a volunteer basis.

Anyway, me pulling away from this is about my situation, not a mark against these techs.

apendua · 2018-03-26T10:46:39Z

@vsivsi @brucejo75 Is there a chance this PR will be merged soon? Thanks for all the work!

antoninadert · 2018-03-27T14:39:33Z

src/gridFS_server.coffee

@@ -258,7 +265,7 @@ if Meteor.isServer
               writeStream.on 'expires-soon', () =>
                  writeStream.renewLock (e, d) ->
                     if e or not d
-                        console.warn "Automatic Write Lock Renewal Failed: #{file._id}", e
+                        console.warn "Automatic Write Lock Renewal Failed: #{file._id.str}", e


Is this intended? I believe it should be _id._str instead of _id.str

antoninadert · 2018-03-27T14:40:21Z

Sorry to jump in, but what is lacking for this to be merged?
By the way, I noticed what is probably a typo in the pull request (unless I am wrong, please tell)

I can't wait to use this project in Meteor 1.6, please tell us

brucejo75 · 2018-03-27T21:10:59Z

thanks @antoninadert, I just pushed a fix for that one.

hluz · 2018-05-08T09:13:51Z

Any chance of merging this soon?

juliomac · 2018-06-20T20:05:47Z

Also looking forward to see this merged!

thumptech · 2018-09-24T01:59:44Z

When merge?

antoninadert · 2018-09-24T10:18:51Z

I think we need a new README to explain which version works with which version of Meteor and then merge.

@brucejo75 What do you think?

edemaine · 2018-12-16T16:37:36Z

FWIW, I just published edemaine:file-collection on Atmosphere. It's almost exactly the current release of vsivsi:file-collection, but with the CoffeeScript code compiled to JavaScript via CoffeeScript v1 (avoiding the need for this PR), and jumping through a few hoops to make it all work; see Github link. This means you can drop it in as a replacement and upgrade your Meteor apps, until this PR gets finished/merged/released. The tests all pass, and I've successfully upgraded my own Meteor app.

brucejo75 · 2018-12-16T18:37:20Z

Thanks @edemaine,

Do you have any interest in taking over this project for @vsiverson, he has a list of todos before he was willing to take the PR. And he is looking for someone to own this project and I am not the right person.

edemaine · 2018-12-17T15:00:13Z

@brucejo75 @vsiverson Yes, I'm willing to help maintain file-collection. It sounds like this PR is done, just needs another review; and then the other to-do items could be turned into issues and addressed by future PRs before release.

vsivsi · 2019-10-08T20:58:00Z

@edemaine @brucejo75 Any interest in maintaining this package if it is transitioned to the Meteor Community Packages organization?

See: https://github.com/Meteor-Community-Packages

My other main meteor package, job-collection, looks to be moved over there, and I'd like to do the same for this one, assuming people are still using it, as I'm no longer an active Meteor developer.

See these discussions for more info:
Meteor-Community-Packages/organization#30
vsivsi/meteor-job-collection#268

edemaine · 2019-10-08T21:04:29Z

@vsiverson Yep. I can try to review PRs at least, and I do have some old plans to add mobile support eventually (which we discussed long ago). Should we open a new issue on https://github.com/Meteor-Community-Packages/organization/issues/?

vsivsi · 2019-10-08T21:08:24Z

I think so. I'm not sure what their workflow is over there. You might ask @mitar for guidance if it's not clear, as he seems to be spearheading the effort for job-collection. Given the shared history of these two packages, it might be smoother to try to do them together.

mitar · 2019-10-08T21:56:23Z

Yes, feel free to transfer packages to me and I will move them over to common organization. Because they are hosted currently on personal GitHub account I think this is the easiest.

mitar · 2019-10-08T21:58:16Z

(Also add communitypackages as a maintainer to Meteor Atmosphere packages, and @meteor-community as a maintainer of any NPM package.)

CS 2.0.3.3, change version to 2.0.0, handle 'this' issue for CS exten…

415b62c

…ded classes, __super__ issue, Interpolated strings issue.

brucejo75 mentioned this pull request Feb 16, 2018

Upgrade to CoffeeScript 2.0 for latest Meteor compatibility (1.6.1) #171

Open

Set default root value, D'oh

b11c938

fix _id problems with coffeescript v2 string interpolation

5b8422c

vsivsi mentioned this pull request Mar 14, 2018

Ported to CoffeeScript 2 vsivsi/meteor-job-collection#268

Open

edemaine mentioned this pull request Mar 26, 2018

Problem with recent but old iOS/macOS devices edemaine/coauthor#294

Closed

antoninadert reviewed Mar 27, 2018

View reviewed changes

repair file._id string reference

fadb214

edemaine mentioned this pull request Oct 8, 2019

New package for inclusion - vsivsi:file-collection Meteor-Community-Packages/organization#33

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade to v2 of coffee #172

Upgrade to v2 of coffee #172

brucejo75 commented Feb 16, 2018

vsivsi commented Feb 16, 2018

brucejo75 commented Feb 16, 2018

brucejo75 commented Feb 16, 2018 •

edited

Loading

vsivsi commented Feb 17, 2018

brucejo75 commented Mar 2, 2018

vsivsi commented Mar 6, 2018 •

edited

Loading

vsivsi commented Mar 6, 2018

edemaine commented Mar 6, 2018

brucejo75 commented Mar 6, 2018 •

edited

Loading

vsivsi commented Mar 6, 2018 •

edited

Loading

vsivsi commented Mar 6, 2018 •

edited

Loading

vsivsi commented Mar 7, 2018

brucejo75 commented Mar 7, 2018

edemaine commented Mar 7, 2018

vsivsi commented Mar 7, 2018

apendua commented Mar 26, 2018

antoninadert Mar 27, 2018

antoninadert commented Mar 27, 2018

brucejo75 commented Mar 27, 2018

hluz commented May 8, 2018

juliomac commented Jun 20, 2018 •

edited

Loading

thumptech commented Sep 24, 2018

antoninadert commented Sep 24, 2018

edemaine commented Dec 16, 2018

brucejo75 commented Dec 16, 2018

edemaine commented Dec 17, 2018

vsivsi commented Oct 8, 2019

edemaine commented Oct 8, 2019

vsivsi commented Oct 8, 2019 •

edited

Loading

mitar commented Oct 8, 2019

mitar commented Oct 8, 2019

Upgrade to v2 of coffee #172

Are you sure you want to change the base?

Upgrade to v2 of coffee #172

Conversation

brucejo75 commented Feb 16, 2018

Changes to package.js

3 areas needed to be addressed:

super and this

FileCollection.__super__ does not exist

String interpolations behave differently

vsivsi commented Feb 16, 2018

brucejo75 commented Feb 16, 2018

brucejo75 commented Feb 16, 2018 • edited Loading

vsivsi commented Feb 17, 2018

brucejo75 commented Mar 2, 2018

vsivsi commented Mar 6, 2018 • edited Loading

vsivsi commented Mar 6, 2018

edemaine commented Mar 6, 2018

brucejo75 commented Mar 6, 2018 • edited Loading

vsivsi commented Mar 6, 2018 • edited Loading

vsivsi commented Mar 6, 2018 • edited Loading

vsivsi commented Mar 7, 2018

brucejo75 commented Mar 7, 2018

edemaine commented Mar 7, 2018

vsivsi commented Mar 7, 2018

apendua commented Mar 26, 2018

antoninadert Mar 27, 2018

Choose a reason for hiding this comment

antoninadert commented Mar 27, 2018

brucejo75 commented Mar 27, 2018

hluz commented May 8, 2018

juliomac commented Jun 20, 2018 • edited Loading

thumptech commented Sep 24, 2018

antoninadert commented Sep 24, 2018

edemaine commented Dec 16, 2018

brucejo75 commented Dec 16, 2018

edemaine commented Dec 17, 2018

vsivsi commented Oct 8, 2019

edemaine commented Oct 8, 2019

vsivsi commented Oct 8, 2019 • edited Loading

mitar commented Oct 8, 2019

mitar commented Oct 8, 2019

Changes to `package.js`

`super` and `this`

`FileCollection.super` does not exist

brucejo75 commented Feb 16, 2018 •

edited

Loading

vsivsi commented Mar 6, 2018 •

edited

Loading

brucejo75 commented Mar 6, 2018 •

edited

Loading

vsivsi commented Mar 6, 2018 •

edited

Loading

vsivsi commented Mar 6, 2018 •

edited

Loading

juliomac commented Jun 20, 2018 •

edited

Loading

vsivsi commented Oct 8, 2019 •

edited

Loading