ReferenceError: moment is not defined (Meteor 1.3)

Question

ReferenceError: moment is not defined (Meteor 1.3)

Closed this issue 9 years ago · 20 comments

Hey remcoder,

With meteor 1.3 the convention is importing dependencies like moment from npm, and not having a global atmosphere package. My file looks like this, and it should. Any way you can accommodate Chronos for 1.3? It's a great package and it would be a shame to not be able to use it with the new style.

import moment from 'moment';
import { Chronos } from 'meteor/remcoder:chronos';

Thanks,
Dylan

Answer 1 · 2016-06-09T14:44:12.000Z

Hi Dylan,

Thanks for reporting. I haven't done much with Meteor 1.3 yet (shame on me!) so that's why this occurs. I will look into it. Do you have a suggested fix by any chance?

cheers

Answer 2 · 2016-06-09T14:54:03.000Z

Hey man! Thanks for the quick reply. I don't know much about authoring packages, but this section of the meteor guide may be what is needed?

Edit: you may have to put that import in a linked 'main module'

Package.onUse(function(api) {
  api.mainModule('my-package.js');
});

Answer 3 · 2016-06-10T21:05:48.000Z

@dylanmcgowan Want to verify this is working before I publish to atmosphere?

Answer 4 · 2016-06-10T21:31:05.000Z

@remcoder sure! How would I go about doing that without grabbing it from atmosphere? Sorry I am a n00b when it comes to packages

Answer 5 · 2016-06-10T21:53:44.000Z

To do that, create a folder called meteor-packages somewhere on your machine. Then, from inside that directoty, clone the github repo. And finally set the PACKAGE_DIRS environment setting to the directory you made:
export PACKAGE_DIRS=<path to dir>

When you run meteor again, it should pick up the local package. That also means that if you want to go back to using the version on atmosphere you must remove the local package.

See here also

No worries if it's too much of a hassle.

Answer 6 · 2016-06-10T22:02:32.000Z

Actually, I'm still fixing some things due to the upgrade to 1.3 so the build is currently failing. You'll want to wait for me to fix that before you test. Sorry about that.

Answer 7 · 2016-06-10T22:04:06.000Z

Ah should be better now :-)

Answer 8 · 2016-06-10T22:21:24.000Z

@remcoder Hey so I have the package running locally, pretty cool how that works. I dont think it's being reactive? I may be wrong. My console isn't throwing errors any more though.

Chronos.liveMoment(this.timeStamp).fromNow();

returns "a few seconds ago" and doesn't live update. this.timeStamp is a Date.now() stored in the db.

Also, how do I disable the PACKAGE_DIRS thing so my versions stay up to date with atmosphere?

Answer 9 · 2016-06-10T22:57:14.000Z

You can stop meteor and close the terminal and then start a new terminal window. The PACKAGE_DIRS setting will be gone then.

It's weird that it wouldn't be reactive. Can you try other Chronos methods? I have a local test application where Chronos.liveMoment works reactively just fine.

Answer 10 · 2016-06-10T22:59:06.000Z

For testing, Chronos.moment().format('ss') would be better as the output from .fromNow() doesn't change for about a minute.

Answer 11 · 2016-06-10T23:18:39.000Z

Hmm i feel super dumb. Sorry for not testing as thorough as I should.. In chromes console, I wrapped the fromNow() in an autorun and it spits out the right time.. Looking at my code i found a spelling mistake with timeStamp instead of timestamp. Such a classic mistake. She's working like a charm :)

Answer 12 · 2016-06-10T23:22:47.000Z

Awesome! Thanks for taking the time to verify it. Appreciated :-)

Answer 13 · 2016-06-10T23:40:25.000Z

No, thank you @remcoder

Answer 14 · 2016-06-10T23:51:36.000Z

I just published v0.4.0 to Atmosphere.

However, I'm not closing this issue b/c I'm running into problems when running unit tests that use Chronos.liveMoment(). This fix might not be that good after all. I'm thinking about removing liveMoment from Chronos, possibly to create a stand-alone package for it. Or maybe I should just add moment as a hard dependency of Chronos, something I've been trying to avoid.

I'll have to think about this..

Answer 15 · 2016-06-15T16:44:43.000Z

may have been as of the version bump, but the global moment variable is undefined for us within the scope of liveMoment? any help would be much appreciated, thanks!

// wrapper for moment.js
function liveMoment(/* arguments */) {
  // only reactively re-run liveMoment when moment is available
  if (!moment) return;
  liveUpdate();
  return moment.apply(null, arguments);
}

ways I've tried to call it:

//same error with date obj & plain unix timestamp
expiryStamp = this.flash.expires.stamp || new Date(this.flash.expires.stamp)

Chronos.liveMoment(expiryStamp).fromNow()
>> Uncaught TypeError: Cannot read property 'fromNow' of undefined(…)

Chronos.liveMoment(expiryStamp)
>> undefined

Answer 16 · 2016-06-15T17:13:53.000Z

How are you including momentjs? As an atmoshere package or as an npm
package?

On Wed, Jun 15, 2016, 18:44 Mike Paszkiewicz notifications@github.com
wrote:

may have been as of the version bump, but the global moment variable is
undefined for us within the scope of liveMoment? any help would be much
appreciated, thanks!

// wrapper for moment.js
function liveMoment(/* arguments */) {
// only reactively re-run liveMoment when moment is available
if (!moment) return;
liveUpdate();
return moment.apply(null, arguments);
}

ways I've tried to call it:

//same error with date obj & plain unix timestamp
expiryStamp = this.flash.expires.stamp || new Date(this.flash.expires.stamp)
Chronos.liveMoment(expiryStamp).fromNow()

Uncaught TypeError: Cannot read property 'fromNow' of undefined(…)
Chronos.liveMoment(expiryStamp)
undefined

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#10 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAcLUvqVVy_WtVPeGFvwE8N_0T-TL5JTks5qMCv7gaJpZM4IyCXI
.

Answer 17 · 2016-06-15T17:22:14.000Z

atmosphere, momentjs:moment - versions match. i haven't switched to the import/export pattern, seems like that may have something to do with it?

Answer 18 · 2016-06-15T20:35:22.000Z

I think the fix for importing moment broke the atmosphere way of including moment. Any global var named moment is shadowed by this code:

let moment;

// if moment is not installed, fine. We don't require it as a hard dependency
try {
   moment = require('moment');
}
catch(e) {
}

That should be any easy fix but on the other hand I don't like the mess that arises. Chronos should now accomodate for 3 situations, being:

momentjs is not installed
momentjs is installed as an Atmosphere package
momentje is installed as an Npm package

This is of course due to the fact that momentjs is not a hard dependeny of Chronos. But I don't want to depend on a library that is way bigger just for the sake of one convenience function.

I think the proper solution would be to remove liveMoment from Chronos and to possibly create a new package just for a reactive version of moment with both momentjs and Chronos as hard dependencies. Not that pretty but definitely more correct.

One big reason why supporting the 3 situations outlined above is difficult, if not impossible, in the long run is that there doesn't seem to be a good way to write automated tests for that. When defining the test in package.js I'll have to choose if and how to depend on momentjs, meaning that there are always 2 scenarios left untested. This is because including packages as a dependency is something that can be done at runtime.

So I'll try to apply an easy fix asap and I'd appreciate everybody advice / stance on the matter for how to handle the situation in the long run.

Answer 19 · 2016-06-15T20:36:04.000Z

tl;dr weak dependencies are evil! Avoid them at all costs :-P

Answer 20 · 2016-06-15T22:08:42.000Z

Fixed for now, although kludgy. It's on Atmosphere as v0.4.2.