Skip to content

Added support for CommonJS/browserify. #261

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Nov 3, 2014
Merged

Added support for CommonJS/browserify. #261

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e29930b
Added support for CommonJS/browserify.
clayzermk1 Sep 16, 2014
28e7551
Fixed a file naming issue from changing the package.json name per #260.
clayzermk1 Sep 16, 2014
87c8b4e
Made use of isObject helper function per feedback.
clayzermk1 Sep 16, 2014
44be740
Added AMD global for plugins per UMDJS and feedback in #261.
clayzermk1 Sep 16, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Gruntfile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ module.exports = function(grunt) {
},
core: {
src: coreFiles.concat(plugins),
dest: 'build/<%= pkg.name %>.js'
dest: 'build/raven.js'
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this because the package.json is now raven-js ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah 😐

I noticed it and since it was from something I suggested, I figured I'd clob it in here. I have it on a separate commit and referenced the issue from the commit message.

},
all: {
files: pluginConcatFiles
Expand Down
2 changes: 1 addition & 1 deletion src/raven.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
// If there is no JSON, we no-op the core features of Raven
// since JSON is required to encode the payload
var _Raven = window.Raven,
hasJSON = !!(window.JSON && window.JSON.stringify),
hasJSON = !!(isObject(JSON) && JSON.stringify),
lastCapturedException,
lastEventId,
globalServer,
Expand Down
17 changes: 13 additions & 4 deletions template/_footer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,18 @@
// Expose Raven to the world
window.Raven = Raven;

// AMD
if (typeof define === 'function' && define.amd) {
define('raven', [], function() { return Raven; });
// AMD
define('raven', function(Raven) {
return (window.Raven = Raven);
});
} else if (isObject(module)) {
// browserify
module.exports = Raven;
} else if (isObject(exports)) {
// CommonJS
exports = Raven;
} else {
// Everything else
window.Raven = Raven;
Copy link
Contributor

Choose a reason for hiding this comment

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

No, we can't do this. It also needs to exist on the global scope for AMD to make plugins work.

So until someone else redesigns those, we can't do this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/umdjs/umd?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Specifically: https://github.com/umdjs/umd/blob/master/commonjsStrictGlobal.js?

Copy link
Contributor

Choose a reason for hiding this comment

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

wtf is that garbage

Copy link
Contributor

Choose a reason for hiding this comment

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

Ugh, I'll accept that if it stops this back and forth argument. So gross though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think anyone using browserify will be fine with https://github.com/umdjs/umd/blob/master/returnExportsGlobal.js which seems a little cleaner and more straightforward. What would you like to support: CommonJS or browserify? From the issues, it seems as though most people are interested in browserify.

}

})(window);