Open tauren opened 7 years ago
Hi Tauren,
Thank you for effort you put into this. I am quite strongly against transpilation within this module, as our baseline target is node v4 and I favour simplicity, however I'd be down for doing something like https://github.com/jeffbski/joi-browser as a new repo/module which acts as a wrapper of sorts around joi
, maybe called generic-pool-browser
(see what I did there :-p ).
How does this sound?
@sandfox
This is your project, so if you'd prefer that you or someone else create and maintain a separate generic-pool-browser
project, that is your prerogative. I can certainly use a wrapping module instead. In fact, I had thought about doing just that so I didn't have to wait on generic-pool
to be updated or to get my PR approved and merged. But I didn't go that way since I assumed you'd prefer to natively support multiple environments and that my wrapping module would ultimately have no purpose.
However, I have to say that I'm not convinced that is the better approach. Is there anything in this module that requires a node environment besides code syntax? It doesn't depend on fs
or other node specific modules, does it?
I totally understand the desire to keep projects simple. But I'm also an advocate for supporting multiple environments if a project lends itself to doing so, as I believe generic-pool
does. I'm currently successfully using my PR branch as an npm link
ed module with my web project. I certainly might be missing something, and if I am please let me know.
Also note that the changes I've proposed don't effect the source at all. It only adds a very simple babel build process that runs when you npm publish
. This means you don't even need to have a dist
folder committed within your repo.
I now see that you are using EventEmitter
from events
. My app has a dependency on eventemitter3
, but that wouldn't be automatically wired up. Perhaps there is more to getting generic-pool
working in the browser. I'll have to look into why things are working in my webapp. I haven't tested my PR outside of my webpack/babel setup, which must be doing something to make it work.
@sandfox Any thoughts on my comments? Should I be looking at creating a generic-pool-browser
project myself? It wasn't clear to me from your response if you wanted to be part of maintaining it.
Hi @tauren, sorry, been busy with very tedious end of company/tax paperwork :-(
I'm totally down for creating / maintaininggeneric-pool-browser
, infact it's here -> https://github.com/sandfox/node-pool-browser (the repo name is like that for consistency with the current one, but the module in npm should defo be called generic-pool-browser
I think I might move the generic-pool stuff into it's own org as it's now at least three repos and could become more (but need to speak to other james about that first).
I'm using
generic-pool
to manage a pool of WebRTC data channels in a web project. This project useswebpack
to bundle assets, which usesuglify
to minify. The problem is that uglify dies on any non-ES5 code.For instance, it dies when it hits the ES6
class
keyword in DefaultEvictor:It would be nice if babel was used to transpile to ES5 before publishing to npm. Both ES5 and ES6 can be published together, as the
package.json
propertiesmain
,browser
, andjsnext:main
can be used to target different environments. This way there wouldn't be any backward compatibility issues.I need this immediately and am willing to work on a PR. Please let me know if you're willing to accept this as a contribution.