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

Fix for moment not working under browserify #25

Merged
merged 1 commit into from
Oct 18, 2011

Conversation

spmason
Copy link
Contributor

@spmason spmason commented Oct 18, 2011

Small fix to the NodeJS detection to make the module work when run client-side using browserify.

I think the way underscore does its client/server-side check best, but didn't want to make that big a change.

@timrwood
Copy link
Member

Sorry, I'm not super familiar with browserify. I'll read up on it today, but maybe you know the answer to the question.

Will this have to change the way Moment.js loads languages?

Currently, Moment.js require()s language files relative to the moment.js source based on keys passed in. Will browserify be able to handle this, or does it require a bigger fix?

https://github.com/timrwood/moment/blob/master/moment.js#L332

@spmason
Copy link
Contributor Author

spmason commented Oct 18, 2011

Hi Tim,

At the moment browserify generates a warning when it sees the language require's and I think it does nothing about it - I'm not worried because I'm not using that functionality.

It should be safe to ignore the warning - browserify people can always load the languages manually (if I understand the code). Maybe browserify has a way of letting you tell it about these modules, but I'm not sure.

timrwood added a commit that referenced this pull request Oct 18, 2011
Fix for moment not working under browserify
@timrwood timrwood merged commit ef1d150 into moment:master Oct 18, 2011
@timrwood
Copy link
Member

Ok, sounds good. If people need browserify and moment.lang, they will have to require the lang files manually using browserify.

timrwood added a commit that referenced this pull request Oct 18, 2011
@timrwood
Copy link
Member

This is up on npm.

ichernev pushed a commit to ichernev/moment that referenced this pull request Feb 6, 2014
Alternative construct moment local to timezone
butterflyhug pushed a commit to WhoopInc/frozen-moment-OLD that referenced this pull request Jul 22, 2014
butterflyhug pushed a commit to WhoopInc/frozen-moment-OLD that referenced this pull request Jul 22, 2014
* feature/2.1.0: (36 commits)
  2.1.0 release
  document that the current time will be used for undefined args with isSame, isBefore, and isAfter
  no longer document Language#months as an array moment#25
  adding note on endof and startof week availability
  addressing moment#42
  fixing format weekday docs
  added a note in the lang docs about using the minified language files rather than the nodejs specific ones
  adding cdnjs note
  switching to rawgit.luolix.top urls instead of raw.github.com urls
  noting cahnge in moment#month to clamp to the end of the target month
  creating duration from asp.net string, fixing duration subtract example
  Building docs
  Duration add, subtract, as, and get docs
  Adding ISO string docs
  Adding docs on AM/PM parsing
  Adding Lang#ordinal token argument
  adding missing doc files
  adding docs for max and min
  adding month string parsing
  adding timezone setter docs
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants