Skip to content

Extend existing moment instance before loading from dependencies#118

Open
Mihailoff wants to merge 2 commits into
icambron:masterfrom
Mihailoff:patch-1
Open

Extend existing moment instance before loading from dependencies#118
Mihailoff wants to merge 2 commits into
icambron:masterfrom
Mihailoff:patch-1

Conversation

@Mihailoff

Copy link
Copy Markdown

In a multi-plugin world require 'moment' leads to problems discussed in #102

In a multi-plugin world `require 'moment'` leads to problems discussed in icambron#102
@Mihailoff Mihailoff changed the title Extend existing moment instance before loading from dependencies #102 Extend existing moment instance before loading from dependencies Jun 28, 2020
Comment thread src/twix.coffee Outdated

# -- MAKE AVAILABLE
return module.exports = makeTwix(require 'moment') if hasModule
return module.exports = makeTwix(if moment then return moment else require 'moment') if hasModule

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

won't this throw if moment hasn't been defined as a global?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In plain JS will work, in CoffeeScript honestly, I don't know. Just double-checked, this code passes tests return module.exports = makeTwix(if moment? then moment else require 'moment') if hasModule

Also, it may have adverse effects on existing users, so technically this should be part of a major "potentially breaking" release.

@Mihailoff

Copy link
Copy Markdown
Author

@icambron fixed

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