dvlsg / async-csp

CSP style channels using ES7 async/await
MIT License
317 stars 18 forks source link

Provide a browser build #13

Closed XVincentX closed 8 years ago

XVincentX commented 8 years ago

It would be great if it would be possible to have a ready to use browser build so we can import this library in codepen directly using npmcdn.

As a source of inspiratin, you might want to look here: https://github.com/XVincentX/async-csp/commit/932fd8c5b25d6f6816a198afa74185e09811bdcc

If that's ok with you, you might want to review/merge this -> #14

dvlsg commented 8 years ago

Is the underlying goal to have a ready-to-use playground which can be shared? If that's the case, then tonic could be used, and has built in async/await functionality.

I know it's not quite the same, but I'm not sure how I feel about going down the road of trying to supply pre-built packages for different environments. There's just no way we could ever cover all of the use cases. I could maybe be convinced for a single browser build, but if the goal is to have a live preview and / or playground, then npm / tonic technically already has us covered.

XVincentX commented 8 years ago

Main problem here is that, without a browser build, I am forced to set up a webpack-browserify-something pipeline even for a complete Es5 application. Furthermore, it would add it's usage in codepen in npmcdn.com as well. That's why I would go for a browser build (as most of libraries already do).

Plus, you can generate those files as part of preinstall part and not have to include them in your repo.

dvlsg commented 8 years ago

Fair enough. Personally, I would strongly recommend some sort of webpack / browserify / systemjs build system, since there's no possible way we could predict each target properly (generators available? promises? IE support?) and a build system can transpile as needed. You'll probably get better performance out of that, as well, with as little transpilation as possible. Things like regenerator are lovely pieces of code, but considerably slower than native solutions like generators.

I'm not opposed to including dist files inside the repo. It's a bit of an anti-pattern, but since we aren't using any native modules / code it's not the end of the world. I believe I'm already doing that, anyways, actually.

If I do decide to go forward with this (I probably will, at some point in the future), I will most likely merge your PR, then go the whole 9 yards and provide es6 / umd / cjs / amd builds with their own subsections, with cjs being the default when using require('async-csp');. I will most likely integrate the system with gulp, though, instead of using just an npm script. You may update the PR to integrate the browserify build with gulp if you're comfortable with that, or leave it as is and I will handle it in the future.

XVincentX commented 8 years ago

I can definitely improve my PR. If you agree with me, the steps would be

  1. Remove the dist directory from git repository
  2. Make it generate from a prepublish script (so it gets into npm package and can be used into npmcdn)
  3. Use webpack to generate the bundle (as it can easily generate a UMD package using its libraryTarget feature)

Let me know what you think!

dvlsg commented 8 years ago

I think I would prefer to see a file structure like:

So all the pre-packaged distribution files can be found from the same root folder. Adding prepublish is a good idea. The dist files can either be kept in git, or removed. I'm leaning towards keeping all of the dist files in the repository for now, though, since we don't utilize any native modules.

My main goal here is to have the following two pieces fully functional with the new build system:

As for the tools that get used internally to those gulp pipelines, I don't really care, as long as they are functional and are reasonably performant. Webpack does seem like a good choice, though, especially with tree-shaking coming up. gulp-webpack may assist with integrating it with the pipeline style task running.

XVincentX commented 8 years ago

An UMD module is compatible with CommonJs, AMD and any other module system. So one build would be enough.

XVincentX commented 8 years ago

@dvlsg I've updated my PR. Try to have a look into!

dvlsg commented 8 years ago

There are technically some scenarios where the magic that UMD attempts to use actually causes odd behavior -- but that's a different discussion. I'll review the PR when I get a moment.

dvlsg commented 8 years ago

Closed via #14, files will arrive with the next npm publish