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

require("assert") is removed from index.js #3

Closed
wants to merge 3 commits into from
Closed

require("assert") is removed from index.js #3

wants to merge 3 commits into from

Conversation

yavuzmester
Copy link

When browserified assert also comes in to the bundle as it is required in index.js. It is not desirable to me.

I removed the require("assert") line and replace the assert call with throw new TypeError.

@coveralls
Copy link

coveralls commented Nov 4, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 01e21dc on yavuzmester:master into 21573d6 on yoshuawuyts:master.

@coveralls
Copy link

coveralls commented Nov 4, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 01e21dc on yavuzmester:master into 21573d6 on yoshuawuyts:master.

@coveralls
Copy link

coveralls commented Nov 4, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 97ea1d6 on yavuzmester:master into 21573d6 on yoshuawuyts:master.

@yavuzmester
Copy link
Author

Even after my change, I see assert module implementation in my bundle.js. It may be caused by browserify itself. But I think it can still be helpful not to use assert there as it is not much needed.

@yoshuawuyts yoshuawuyts closed this Nov 6, 2016
@yoshuawuyts
Copy link
Owner

nah, let's not do this - use unassertify if you want to strip out asserts

On Fri, Nov 4, 2016 at 4:47 PM yavuzmester notifications@github.com wrote:

Even after my change, I see assert module implementation in my bundle.js.
It may be caused by browserify itself. But I think it can still be helpful
not to use assert there as it is not much needed.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#3 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACWlesyyweqLwsbt2Eq1jx4yllNF7_pyks5q6uL8gaJpZM4KpPy4
.

@yavuzmester
Copy link
Author

ok, cool

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.

None yet

3 participants