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

change condition of using node export system. #41

Merged
merged 2 commits into from
Dec 26, 2014
Merged

change condition of using node export system. #41

merged 2 commits into from
Dec 26, 2014

Conversation

ironhee
Copy link
Contributor

@ironhee ironhee commented Dec 22, 2014

The original condition of using node export system causes error when used with QUnit.

QUnit's 'module' Function is defined globaly.
So current export logic is malfunctioning.

I change that logic more precise.

The original condition of using node export system causes error when used with QUnit.
if (typeof module !== 'undefined' && module.exports) {
exports = module.exports = JsDiff;
}
exports.JsDiff = JsDiff;
Copy link
Owner

Choose a reason for hiding this comment

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

This materially changes the export. Why creating the named, circular export?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Owner

Choose a reason for hiding this comment

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

Having gone this long without anyone complaining (and my sense is that node has won out), I'd rather not add this API as it effectively makes the API conditional on the environment. If you remove this line I'm fine merging the rest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay I'll fix that line~ :D

@ironhee ironhee closed this Dec 26, 2014
@ironhee ironhee reopened this Dec 26, 2014
kpdecker added a commit that referenced this pull request Dec 26, 2014
change condition of using node export system.
@kpdecker kpdecker merged commit a33116c into kpdecker:master Dec 26, 2014
@kpdecker
Copy link
Owner

Released in 1.2.1

@ironhee
Copy link
Contributor Author

ironhee commented Dec 27, 2014

I am proud of contributing your repo.. thank you!

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