browserify / commonjs-assert

Node.js's require('assert') for all engines
MIT License
293 stars 57 forks source link

Expose strict mode functionality #41

Closed lukechilds closed 5 years ago

lukechilds commented 5 years ago

This PR adds support for using assert in strict mode.

Resolves: #37 Makes some progress towards: https://github.com/browserify/commonjs-assert/issues/32

Ported from this Node.js PR: https://github.com/nodejs/node/pull/17002

Tests are all failing. This isn't due to any changes in this PR they are already failing in master: https://github.com/browserify/commonjs-assert/issues/33

I've tested locally on a couple of different Node.js versions and tests are passing but would feel a lot more confident getting this properly tested against all browser targets.

Happy to look into the Travis issue and submit a separate PR resolving that if you want.

lukechilds commented 5 years ago

@goto-bus-stop addressed feedback, let me know if requireing the ponyfill is ok.

Also resolved some of the issues with the tests here: https://github.com/browserify/commonjs-assert/pull/42

goto-bus-stop commented 5 years ago

I think the buffer shim is inlined because recent buffer versions don't work at all on old IE (throws an error on load). object-assign should be ok, it seems like assert only supported IE9+ already… i'll have a closer look later to confirm what the current oldest supported version is and document it somewhere :P

lukechilds commented 5 years ago

I think the buffer shim is inlined because recent buffer versions don't work at all on old IE

Ahhhh, that makes sense.

i'll have a closer look later to confirm what the current oldest supported version is and document it somewhere :P

That would be great, thanks!

lukechilds commented 5 years ago

@goto-bus-stop is this ok to be merged?

lukechilds commented 5 years ago

@goto-bus-stop should we publish a new release with this?

I don't have npm permissions.

goto-bus-stop commented 5 years ago

ideally yes! i don't have publish perms either :'

@defunctzombie do you have a moment to give the browserify org members publish access on npm?

npm access grant read-write browserify:developers assert
lukechilds commented 5 years ago

(or @calvinmetcalf or @solderjs) 🙏

lukechilds commented 5 years ago

Any update on this?

Ping @defunctzombie @calvinmetcalf @solderjs

goto-bus-stop commented 5 years ago

:package: 1.5.0 :tada: