-
Notifications
You must be signed in to change notification settings - Fork 87
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
Minor changes for basic browser compat #15
Conversation
Is there a way we can do this without modifying the code? For example, this change seems fine, but I cannot make it without tests included here that fail without it and pass with it, otherwise I can pretty much guarantee it'll be forgotten about and some release will just break it again. Is there something I can add to |
Do the tests not pass for you? I ran the tests and they pass for me. I didnt run them in the browser tho.... There is a way to totally not run the module using browserify, but that means that we don't get the depreciation warnings in the browser code. Which is the whole point, lol. I don't know of a way to use this without getting it to actually run in the browser. I am running this in code i am prepping for our production environment and it seems to be working fine. I can let you know if it does once we get some load on it. |
The tests pass just fine. What I'm saying is that say I accept this PR and add another
Let me know. How are you getting this module into the browser, though? Are you using browserify or something else? Browserify makes it simple to refactor the written location such that it can be injected into this module and it can write to different locations based on that, which would be a better solution. |
|
Soooo, sad to say, this is not the only issue I ran into. |
Ah. For reference, there was issue #14 , but that was a simply fix: the |
Yeah, no worries. When I got it working in Chrome so easily I thought it might be good to go. And if I cant use it in both sides of my app then I would rather write something simple that i can use on both. And I promise now that we have launched our app I will have time to integrate some browser test for the express router. |
Totally understand if you don't want to take this, but this make the code atleast RUN in chrome. And it shouldn't effect node execution at all. Anyway, thoughts?