-
-
Notifications
You must be signed in to change notification settings - Fork 57
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
Remove explicit util dependency. #36
Conversation
yeah any idea why we aren't using the browserify util ? |
no clue. i think we should use the builtin instead, i take it you do too? i'll update the PR. |
`assert` depended on the userland `util` module rather than browserify's builtin, which could cause two different versions of `util` could be included in bundles, if browserify's version doesn't exactly match `assert`'s dependency. Since `util` is builtin in Node and in bundlers, we can safely use it instead of explicitly depending on it.
well this is me trying to remember why it was done in the first place, there could have been a reason |
ah, i see 😅 the commit that introduced it doesn't have that much context: ea3de89 i suppose the point is to guarantee behaviour in |
@calvinmetcalf does this seem ok? I can't personally think of a reason not to do it. I'd like to upgrade the |
What if I want to |
that's a fair point, in webpack 5 and some other default configs node shims aren't included by default. i didn't think of the case where you manually install |
It's probably hard to figure this out for the browser. If we are would be able to tree shake after running some code, it would be possible to run some feature tests and only load |
Should this stay upon? |
no lets use the newer version from #44 |
assert
depended on the userlandutil
module rather than browserify'sbuiltin, which could cause two different versions of
util
could beincluded in bundles, if browserify's version doesn't exactly match
assert
's dependency.Since
util
is builtin in Node and in bundlers, we can safely use itinstead of explicitly depending on it.