GoogleCloudPlatform / stackdriver-errors-js

Client-side JavaScript exception reporting library for Cloud Error Reporting
https://cloud.google.com/error-reporting/
Apache License 2.0
362 stars 54 forks source link

Expose as UMD module and use browserify #53

Closed bz2 closed 5 years ago

bz2 commented 5 years ago

Changes

Use UMD for stackdriver-errors module.

Drop special handling for StackTrace object being undefined.

Change gulp dist task from concatting prebuilt source to building from components and bundling using browserify.

Supply a separate fills.js with polyfills from core-js, based on the upstream polyfills in stacktrace-js. This may be overly conservative as browsers back to IE 9 have good enough ES5 support, just filling Promise is probably be reasonable.

Include only the common package files and those built under dist/ in the npm package.

Notes

This does solve the basic issue wanting to both import the library into ES6 packages, and having a plain javascript file that can be used in the browser, but is likely not the final state.

For now, I've thrown the UMD declaration in the source, but it probably makes sense to switch to a CommonJS module and add the wrapper using browserify, and make the other pieces depend on that built copy.

I also tried a branch that builds using webpack instead of gulp+browserify. Main reason for proposing this version instead is the nice bits of webpack work best with ES6 style imports, which none of our dependencies use. So, ends up with a bigger and less streamlined bundle than the older tools manage.

Have a followup branch that switches to running tests with mocha directly and using eslint.

bz2 commented 5 years ago

Note, this does change to default import style, to match the way the global is exported, but I'm not sure if it's the right thing.

Before:

import * as StackTrace from 'stacktrace-js';
import { StackdriverErrorReporter } from 'stackdriver-errors-js';

window.StackTrace = StackTrace;
const errorHandler = new StackdriverErrorReporter();

Now:

import StackdriverErrorReporter from 'stackdriver-errors-js';

const errorHandler = new StackdriverErrorReporter();

Could change back to the current non-default style.

See also https://github.com/bz2/stackdriver-errors-js/commit/d1862e88ca2e08f6945a61dd277ea11336fda812 for example making the demo into an ES6 module.

steren commented 5 years ago

Thanks.

Can you resolve the conflicts?

Also, I think some example of the README could be updated to use. For example, the "Best Practices" parts.

ddykhoff commented 5 years ago

Any update on this? Would love to use this as a module

steren commented 5 years ago

Sorry I want to take the time to review properly, and I was away from keyboard in the past weeks.

bz2 commented 5 years ago

Likewise, I think there are a few tweaks that can be made here to make the proposal better but was enjoying a longer than normal holiday break. :)

bz2 commented 5 years ago

@steren Have made that change, base library is now commonjs (uses require()) and the built copy only includes the UMD junk.

ddykhoff commented 5 years ago

When is the next planned npm publish for this?

steren commented 5 years ago

Thanks for the reminder, I'll try to do this this weekend after some testing

bz2 commented 5 years ago

@ddykhoff @steren I have one more change in my list that I've implemented but not polished and proposed yet, will try to get to that this evening, then also do some testing.