-
Notifications
You must be signed in to change notification settings - Fork 62
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
errors are not necessary in some cases #74
Comments
Specifically, I see /***/ (function(module, exports, __webpack_require__) {
"use strict";
/* WEBPACK VAR INJECTION */(function(process) {
var blacklist = [
'freelist',
'sys'
];
module.exports = Object.keys(process.binding('natives')).filter(function (el) {
return !/^_|^internal|\//.test(el) && blacklist.indexOf(el) === -1;
}).sort();
/* WEBPACK VAR INJECTION */}.call(exports, __webpack_require__(1)))
/***/ }), |
I think the idea here is that process.binding is something that ONLY applies to node, not to browser code so that if it is ever called in the library you are almost certainly attempting to use a module that won't work and thus should raise an error. What is the module that raising the errors ? |
It's not my code, I cannot control it, it might be a core module, or an NPM module, not sure which. I can check. But it does come from the code I pasted above. |
@calvinmetcalf if you think about it, the developer using your library can carry the responsibility of checking if the code is running in the browser or node.js, or wherever. the developer can call: if(isNode){
process.binding() // etc etc
}
else{
// we are in browser
} throwing an error doesn't help anyone - it gives the developer using your polyfill less power/less control. The developer using your library is smart enough to not call So with that in mind, for code that I cannot control, it would be best to mimic Node.js as much as possible. |
You are trying to use in a browser a module called builtin-modules, you 100% do not want to bundle this up for use in the browser as it's just a list of of the paths on your hard drive where you can find the builtin-modules. |
@calvinmetcalf I might be be able to avoid bundling builtin-modules, but right now the call to builtin-modules is in my codebase and will get bundled by webpack. ultimately, my problem will be solved if you were to accept the PR changes. I am not sure if it would cause others problems. |
@calvinmetcalf my use case is different, this is not for a web application. I am trying to get a library that was heavily written for backend use, into the browser. Unfortunately, I can't really rewrite my library to make it more browser friendly. For my use case, the better way to do it is to shim things so that they are I might just have to steal the code and adapt it for my use, that's no big deal. From a philosophical standpoint however, I strongly believe it makes no sense to throw errors for functionality that does not exist in the front-end. Just make it a noop. It literally helps nobody to throw an error. IMO. |
ok so my reasoning is that most every situation I can think of code that accessed bindings is an error that you didn't mean to include and having an early error helps because it lets you know oops I didn't mean to include this file in my bundle. making a fork that does something unique is something I've done myself, you could probably also just monkey patch this library. out of curiosity what exactly are you trying to bundle, there may be a significantly easier way to do what you are trying to do. |
I am trying to bundle a test library - https://github.com/sumanjs. It's similar to AVA, it's heavily backend (node.js) oriented, but I know that it can be lifted into the browser with the right polyfills, but I may have to write most of those polyfills myself. For example, there apparently is no good fs polyfill, but I know that I can write one that will fit my use case. Fortunately, people using my test library in the browser, probably won't need the same polyfills, so they won't clash. I am not a big believer in unit tests that run in the browser, now that many cross-browser incompatibilities no longer exist as much. So I wrote Suman to be backend oriented - the best way to test browser is to use Selenium etc, instead of unit tests that run in the browser. But now I realize it would be a nicety for suman to be able to run in the browser, so here I am trying to retrofit it a bit. Instead of rewriting a lot of code, better to just polyfill/shim backend utilities such as fs. It's a pretty fun task to try to browserify the damn thing actually. |
… On Wed, Apr 5, 2017 at 5:07 PM Operations Research Engineering Software+ < ***@***.***> wrote:
I am trying to bundle a test library - https://github.com/sumanjs. It's
similar to AVA, it's heavily backend (node.js) oriented, but I know that it
can be lifted into the browser with the right polyfills, but I may have to
write most of those polyfills myself. For example, there apparently is no
good fs polyfill, but I know that I can write one that will fit my use case.
Fortunately, people using my test library in the browser, probably won't
need the same polyfills, so they won't clash.
I am not a big believer in unit tests that run in the browser, now that
many cross-browser incompatibilities no longer exist as much. So I wrote
Suman to be backend oriented - the best way to test browser is to use
Selenium etc, instead of unit tests that run in the browser.
But now I realize it would be a nicety for suman to be able to run in the
browser, so here I am trying to retrofit it a bit. Instead of rewriting a
lot of code, better to just polyfill/shim backend utilities such as fs.
It's a pretty fun task to try to browserify the damn thing actually.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#74 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABE4n-SED-2vMByjX7WOvlht5jxVxkahks5rtAKtgaJpZM4MzzX0>
.
|
@calvinmetcalf I tried that I believe, I don't remember it working for me, but will take another look though |
What I actually will want to do is patch fs for the browser and make most of the methods on fs no-ops. Looks like browserify-fs is attempting to fill in some of the functionality of fs, but I don't need that. |
@calvinmetcalf this is where I could use your help though - if you look towards the bottom, what I am looking to do is use Webpack and plug-in my own polyfills instead of using the "official" ones. If you have any idea how to do that, please lmk. |
I think there is a specification error in the code
It would be much better if
process.binding()
andprocess.chdir()
were no-ops.Throwing an error literally just breaks things, there is no point in polyfilling functions and just throwing an error, it doesn't make any sense. If the behavior is not supported in the browser just make it a noop. I hope this makes sense to you!
As you can see, in your existing code
process.cwd()
is pretty much a no-op even though it's not supported in the browser really either.The text was updated successfully, but these errors were encountered: