-
Notifications
You must be signed in to change notification settings - Fork 39
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
Fail gracefully in unsupported browsers #18
Comments
Hi! What environment are you looking to have a fallback in? Would you be willing to make a PR? If not a PR, then please let me know enough information so I can make the appropriate code :) |
As per #14, in Firefox it throws this error:
I will gladly make a PR, but wasn't sure exactly what I should be checking against. Is it sufficient just to check for I am yet to come across an environment which doesn't have a |
Hi! Can you say an exact version of Firefox I need to target as well? For example, do I need to ensure it worked on Firefox 3.5 or is some other version more appropriate? The stack property is a string. Different environments format their string differently and the format is not standard. This means you're just going to have the same incompatibilities with the stack property. We actually interrogate the stack at a level that the stack string just really does not work. The stack string also makes it virtually impossible to use if a file name contains a new line character (yes, file names can contain a new line, so you cannot just split the stack on "\n"). I hope that helps explain why this module can not use the string stack property. |
Oh, and yet another issue is that the .stack property only contains a portion of the stack. This means if what we are looking for is too far on the stack, it won't even be contained in the .stack property. Essentially this module just cannot function at all without the V8 stack functions (or something similar). What makes this module great is that it will include the exact callsite of the usage in your code on warning, it will not warn on even usage (think about a webserver that uses a deprecated method for a request--that would mean one message per request!), it will also ensure that you are warned for each unique usage in your code, not just stop warning after the first use. This allows people to run a test suite once and get all the warning they need to fix, not run once, get one warning, fix, run again, get second warning, fix, etc. |
Also, on this point, it actually is not our responsibility, as you can see from the package.json, we have clearly outlined out supported environments. Our contract with users is this code is only to be used in Node.js 0.6+; any other use is at you own risk. |
Also, it may be useful another user attempted this before in #15 but after they dug into the code, abandoned the PR. Just pointing you to the PR so essentially you can read it and pick up where it was left off :) |
I'm using Firefox 35.0, any reasonably recent version would be fine. Yes I love the single-warning and proper callsite, it makes my life so much easier :) Thanks for the info on |
Hi @SystemParadox , I'm looking into this, and have another question for you: how are you getting this code into the web browser in the first place? Are you just copy-pasting the code, or using a system like browserify or webpack (and if so, which one specifically)? |
I am using browserify.
|
Awesome, thanks, @SystemParadox ! So what I'm going to do for now, while #16 is underway, is get this module to just work in a "noop" mode when in the browser. I was thinking of doing this simply with How does that sound :) ? |
Ok, |
Hi @SystemParadox , any feedback on the current |
Sorry I'm away at the moment, will have a look at this next week. |
I can remove my shim and it still runs in Firefox :) However, in the longer term I don't think you need to use a package.json shim. Just check for the availability of the V8-specific APIs (Error.captureStackTrace, etc) before you use them. If they are not available, then noop or fallback to alternative implementation. It worked fine in Chrome before, by using a package.json shim we've lost that. |
I understand and this is only the short-term fix so that people's packages run in Firefox at all. I am working on a more robust one. |
Until #16 is implemented, please please can we at least have graceful failure when running in unsupported browsers?
I don't mind whether you want to print a warning every time, never print a warning, or use
console.error
, but I would like the rest of my code to run. At the moment depd blows up with an exception and everything stops :(In reference to #14, defining a fallback isn't a good solution at all:
depd
does non-standard things, not meThanks.
The text was updated successfully, but these errors were encountered: