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

Establish the root object, `window` in the browser, or `global` on the server. #342

Merged
merged 3 commits into from Jul 5, 2012

Conversation

Projects
None yet
3 participants
@p-baleine
Contributor

p-baleine commented Jun 20, 2012

Instead of using window object directly, establish the root object which will be window in the browser or global on the server.

The change imitated underscore.js.
This change would also make moment.js possible to use on mongoDB shell.

Instead of using `window` object directly, establish the root object …
…which will be `window` in the browser or `global` on the server.
@rockymeza

This comment has been minimized.

Show comment
Hide comment
@rockymeza

rockymeza Jun 20, 2012

Contributor

This looks acceptable to me @timrwood, what do you think?

Underscore, Backbone, and jQuery all provide a noConflict method too. Do you think this is important at all? Does anybody ever run two versions of the moment library?

Contributor

rockymeza commented Jun 20, 2012

This looks acceptable to me @timrwood, what do you think?

Underscore, Backbone, and jQuery all provide a noConflict method too. Do you think this is important at all? Does anybody ever run two versions of the moment library?

@timrwood

This comment has been minimized.

Show comment
Hide comment
@timrwood

timrwood Jun 20, 2012

Could this just be this.moment = moment?

Could this just be this.moment = moment?

This comment has been minimized.

Show comment
Hide comment
@p-baleine

p-baleine Jun 21, 2012

Owner

Yes, it could be. In this change, moment is assigned to global object in the same way as underscore.
I think that explicitly establishing root object improves readability. What do you think?

Owner

p-baleine replied Jun 21, 2012

Yes, it could be. In this change, moment is assigned to global object in the same way as underscore.
I think that explicitly establishing root object improves readability. What do you think?

This comment has been minimized.

Show comment
Hide comment
@timrwood

timrwood Jun 21, 2012

If we are only using root one place, it may make sense to just leave it out and move the line 20 comment to line 1086.

If we are only using root one place, it may make sense to just leave it out and move the line 20 comment to line 1086.

@timrwood

This comment has been minimized.

Show comment
Hide comment
@timrwood

timrwood Jun 20, 2012

Member

I think the noConflict is more because they use a single character namespace, so there were conflicts between jQuery and prototype (or some other library that used $). I don't think there will be conflicts with moment as a top level namespace, but if there are, we can address it then.

@p-baleine, have you tested this in mongoDB yet? I'm not too familiar with the platform...

Also, I remember taking this out with another change but I don't remember why. I think it has something to do with browserify, see #25. I'll do some more research about browserify today to see if this change would affect this.

Member

timrwood commented Jun 20, 2012

I think the noConflict is more because they use a single character namespace, so there were conflicts between jQuery and prototype (or some other library that used $). I don't think there will be conflicts with moment as a top level namespace, but if there are, we can address it then.

@p-baleine, have you tested this in mongoDB yet? I'm not too familiar with the platform...

Also, I remember taking this out with another change but I don't remember why. I think it has something to do with browserify, see #25. I'll do some more research about browserify today to see if this change would affect this.

@p-baleine

This comment has been minimized.

Show comment
Hide comment
@p-baleine

p-baleine Jun 21, 2012

Contributor

@timrwood, sorry I'm not familiar with browserify.
In mondoDB shell, moment which applied this change works normally.
I wrote sample code here.
In this code, moment is used for truncation of date.

Contributor

p-baleine commented Jun 21, 2012

@timrwood, sorry I'm not familiar with browserify.
In mondoDB shell, moment which applied this change works normally.
I wrote sample code here.
In this code, moment is used for truncation of date.

timrwood added a commit that referenced this pull request Jul 5, 2012

Merge pull request #342 from p-baleine/develop
Establish the root object, `window` in the browser, or `global` on the server.

@timrwood timrwood merged commit 73c3363 into moment:develop Jul 5, 2012

@timrwood

This comment has been minimized.

Show comment
Hide comment
@timrwood

timrwood Jul 5, 2012

Member

Merged in, thanks for contributing!

Member

timrwood commented Jul 5, 2012

Merged in, thanks for contributing!

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