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

Projects
None yet
2 participants
@spmason
Contributor

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

This comment has been minimized.

Show comment
Hide comment
@timrwood

timrwood Oct 18, 2011

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

Member

timrwood commented Oct 18, 2011

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

This comment has been minimized.

Show comment
Hide comment
@spmason

spmason Oct 18, 2011

Contributor

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.

Contributor

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

Merge pull request #25 from spmason/patch-1
Fix for moment not working under browserify

@timrwood timrwood merged commit ef1d150 into moment:master Oct 18, 2011

@timrwood

This comment has been minimized.

Show comment
Hide comment
@timrwood

timrwood Oct 18, 2011

Member

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

Member

timrwood commented Oct 18, 2011

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

This comment has been minimized.

Show comment
Hide comment
@timrwood

timrwood Oct 18, 2011

Member

This is up on npm.

Member

timrwood commented Oct 18, 2011

This is up on npm.

ichernev pushed a commit to ichernev/moment that referenced this pull request Feb 6, 2014

Merge pull request moment#25 from hurrymaplelad/parse-local
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

Merge branch 'feature/2.1.0' into gh-pages
* 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 rawgithub.com 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