Skip to content
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

Do not log error when requiring moment-timezone multiple times in Node #231

Closed
mderazon opened this issue Jul 7, 2015 · 42 comments
Closed

Comments

@mderazon
Copy link

mderazon commented Jul 7, 2015

In Node it is quite common to require a module in multiple places

Right now if I require moment-timezone in multiple places I am getting

Moment Timezone 0.4.0 was already loaded with data from 2015d

That comes from this line (#212 )

This shouldn't be logged as an error and just clutters the server logs.
If anything, it should be a warning but in Node env it shouldn't be printed at all IMO

@mattjohnsonpint
Copy link
Contributor

Except that it can actually cause errors when the same time zone is defined more than once.

@timrwood - Perhaps we need to do more than just log an error in order to accommodate node.js scenarios?

@mderazon
Copy link
Author

mderazon commented Jul 9, 2015

AFAIK node has a caching mechanism when requiring modules so that it shouldn't load module more than once

https://nodejs.org/api/modules.html#modules_caching

@mattjohnsonpint
Copy link
Contributor

If that's the case, then why would you be seeing the error message in your logs? It should only appear then if it wasn't already loaded in the cache, right?

@mattjohnsonpint
Copy link
Contributor

Perhaps you could supply a GIST with a minimal example that gives the error?

@mderazon
Copy link
Author

I can't seem to be able to reproduce this.

I have a setup where I require moment-tz couple of times in my app and I also require a second module that requires moment-tz. When I try to replicate it I don't get the error message.

It looks like some quirk in npm that i'm not aware of, I will try to investigate it further and understand why it's being loaded twice in my app

@mderazon
Copy link
Author

@mj1856 problem solved.

I had couple of old dependencies in my node_modules folder. After cleaning it up and restarting the module seems to be loading only once.

Sorry for the noise,

Thanks for your help !

@mattjohnsonpint
Copy link
Contributor

No problem, and glad to hear you got it working! 😄

@FredKSchott
Copy link

We're still having this problem. We use moment-timezone inside of our app, but the ORM sequelize also uses moment-timezone internally. So we get this error.

What exact problems come from two different apps requiring moment? Can that problem still occur when a moment-timezone is loaded inside both the application and a dependency?

@mattjohnsonpint
Copy link
Contributor

@timrwood - Is there a better way to handle this? Loading into two different instances should be ok. It's only loading twice in the same instance that is problematic. Maybe just make the loading idempotent instead of throwing an error?

@jonahkirangi
Copy link
Contributor

I seem to be running into the same issue as @FredKSchott. Any discussion regarding two different apps requiring moment?

Moment Timezone 0.3.1 was already loaded with data from 2015a

@poislagarde
Copy link

Same here, any suggestions?

@antoniopironti
Copy link

+1

Making the loading idempotent as suggested by @mj1856 would be ideal imo.

@johnRivs
Copy link

I went from 0.3.1 to 0.5.0. I'm requiring moment-timezone literally in a single file. Removed node_modules, did npm cache clean, then installed 0.5.0. I get Moment Timezone 0.3.1 was already loaded with data from 2015a.

How do I solve this?

@Braynid
Copy link

Braynid commented Feb 22, 2016

I'm getting this as well. Sequelize has moment-timezone as a dependency and almost certainly that causes the issue.

@jhohlfeld
Copy link

+1

Making the loading idempotent as suggested by @mj1856 and @antoniopironti would be ideal imo.

@loris
Copy link

loris commented Feb 24, 2016

Same issue here, let’s say I have an app using moment, and dependencies using moment-timezone (like Sequelize or Agenda), it displays Moment Timezone 0.3.1 was already loaded with data from 2015a

@ghost
Copy link

ghost commented Mar 1, 2016

I'm able to reproduct this problem if i install the package rschmukler/agenda, because it depends on [email protected] and [email protected] -> which in turn depends on [email protected].

@igormucsicska-planorama
Copy link

+1

@djtregear
Copy link

Hi I'm also getting "Moment Timezone 0.3.1 was already loaded with data from 2015a" when I require('agenda') from rschmukler/agenda.
It doesn't appear to be causing me any issues, just a little annoying.

@timrwood
Copy link
Member

timrwood commented Mar 4, 2016

I've started work on #298, breaking moment-timezone into smaller es6 modules, which should make this problem easier to address.

The main problem is that the data loaded in an older version does not match the data loaded in a newer version.

I suppose we could compare the moment-timezone version and the tzdb versions and use the newest one.

@ee99ee
Copy link

ee99ee commented Apr 7, 2016

Also getting this...

@niftylettuce
Copy link

still not fixed?

@loris
Copy link

loris commented May 27, 2016

It’s been almost a year, maybe we could remove the console.log polluting logs in the meantime?

@avingochea
Copy link

+1

@stephengardner
Copy link

Another month

@dawsonc623
Copy link

dawsonc623 commented Jul 5, 2016

This is a pretty big pain for me, as the logging library used on an e-commerce project I am tasked with fixing suffers from this issue, so none of the logs have dates or times. It is insanely difficult for me to track down which logs occurred when to match logs to observed behavior in the system at the time they were observed.

At this time, I am reduced to sitting and waiting for the error to happen again to see exactly which logs are written, which means I am literally waiting for this company to lose more money. Can this be prioritized in any way? If you have not completed a fix, but have completed your reorganization, can it be placed in a branch so that the community (myself included) can try to complete the fix ourselves without stepping on your work?

I would also consider this a bug versus an enhancement; it prevents usage.

@loris
Copy link

loris commented Jul 7, 2016

Seriously, just remove the useless console.log line. I can’t even do it in a fork, moment-timezone is a dep of other libraries i’m using. Here’s what i have to do right now, this is ridiculous

const oldConsoleLog = console.log;
console.log = () => {};
import Agenda from 'agenda';
console.log = oldConsoleLog;

@jonnywyatt
Copy link

This is polluting our logs, could you change from 'error' to 'warning' please?

@ronenteva
Copy link

+1

5 similar comments
@damienleroux
Copy link

+1

@dimd13
Copy link

dimd13 commented Sep 12, 2016

+1

@Left47
Copy link

Left47 commented Sep 13, 2016

+1

@raabbajam
Copy link

+1

@nmaggioni
Copy link

+1

@aletorrado
Copy link

+1 !!!!

@ptitdje45
Copy link

+1

@rollrodrig
Copy link

Hi. i saw why this error is log.
When i load
moment.js
moment-timezone.min.js
moment-timezone-with-data.min
print the error.
Also with
moment.js
moment-timezone.min.js
moment-timezone-with-data.min.js
moment-timezone-with-data-2010-2020.min.js

To prevent the log error i only loaded
moment.js
moment-timezone-with-data.min.js

And now the error is gone.

Greetings.

@basicdays
Copy link

I think the problem is NPM 3's deduplication and moment-timezone's monkey patching of the moment package. I'd have to test this, but I think the following causes multiple different moment-timezone packages to attempt to monkey patch the same moment module.

my-project/
|- package.json
|- node_modules/
   |- moment/ (v2.6.0)
   |- moment-timezone/ (v0.5.1)
   |- do-stuff/
      |- package.json (can use the same moment version but a different moment-timezone)
      |- moment-timezone/ (v0.4.1)

So when do-stuff requires moment-timezone, it attempts to monkey patch the root moment package. And when moment-timezone is required in my-project, it attempts to monkey patch moment with a newer moment-timezone package even though the do-stuff library already monkey patched it with an older moment-timezone.

I'd have to think of some ways that could be done to solve this, but this isn't an easy fix. If the projects were merged, the problem would probably go away though...

@dilame
Copy link

dilame commented Dec 14, 2016

This is just horrible.

my app just had broken because of this. I installed new package sails-hook-cron, that uses [email protected] , and other part of my application no longer able to call moment.tz.guess().
First i thought it is nodejs issue. But now i see its smth wrong with moment-timezone. Fix it, please.

@maggiepint
Copy link
Member

The console.log has been removed and released 0.5.11

@nitrocode
Copy link

I just saw this from pm2-logrotate's error logs

Moment Timezone 0.5.14 was already loaded with data from 2017c

@Dushusir
Copy link

now,it's

Moment Timezone 0.5.14 was already loaded with data from 2017c.

what should i do?

load js:
<script src="plugins/moment.min.js"></script>
<script src="plugins/moment-timezone.min.js"></script>
<script src="plugins/moment-timezone-with-data.js"></script>
<script src="plugins/moment-msdate.js"></script>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests