vsivsi / meteor-job-collection

A persistent and reactive job queue for Meteor, supporting distributed workers that can run anywhere.
https://atmospherejs.com/vsivsi/job-collection
Other
385 stars 68 forks source link

Replacing mrt:later with npm package #198

Open tcastelli opened 8 years ago

tcastelli commented 8 years ago

Hello, not sure if this has been proposed/tried before, but when deploying meteor apps including this package i get some warnings on the client about mrt:later.

Would it be possible to use later npm packages directly?(mrt:packages are unsupported anyways) https://www.npmjs.com/package/later or https://www.npmjs.com/package/meteor-later

vsivsi commented 8 years ago

Hi, yes, it should be possible to use the new npm package support to load later.js directly. I'll look into it! Thanks for reporting this.

benlavalley commented 7 years ago

I've had job-collection cloned locally for a while as I've tweaked it to only run on the server (reducing my browser client size payload) and I tweaked it to use the NPM Later package.

Put these lines in packagejs inside Package.onUse: Npm.depends({ "later": "1.2.0"}); api.use('ecmascript', ['client','server']);

Had to add the ecmascript line so that 'module' is defined as the import command appears to be transpile to that.

Added this line to src\shared.coffee to do the import: import { default as later } from 'later'

Things seem to work OK. I spent a good bit of time figuring this out as I had to do something similar for the tap:i18n package which also uses coffeescript and I had to update to use with NPM packages (specifically, the new simple-schema from aldeed).

I am not an actual programmer so take this with a grain of salt :)

vsivsi commented 7 years ago

Thanks for the research into this. Needless to say, this task hasn't bubbled up to be my highest priority, but I'll try to work this into the next update.

benlavalley commented 7 years ago

I don't know if the Npm depends thing I mentioned is the best approach - I think using another technique (npm link?) is supposed to be better. Ran into an issue with another package where simpl-schema was throwing errors that babel-polyfill was already loaded, and I guess it was because it was trying to load it's own via NPM despite the fact that it was already supposed to be loaded globally... Some related links that might be useful:

https://github.com/meteor/meteor/issues/5890

https://forums.meteor.com/t/npm-depends-on-a-local-npm-package/9177/6

mitar commented 7 years ago

@benlavalley, you mixes two ways of using NPM packages together.

If we need only a server-side package, then probably it is still better to just use Npm.depends in package.js and then Npm.require to load it (not ecmascript import). This will pin the dependency exactly on the version, and Meteor will install the NPM package automatically.

Alternatively, you could use ecmascript import, but then the user of this package would have to install later.js in their app in root and add it to app's package.json. A package can then use a check and display a warning if such NPM package is not installed (and/or gracefully degrade in functionality).

Personally I think the first approach would be better because we need NPM package only on the server side.

vsivsi commented 7 years ago

@mitar I concur.