Skip to content
This repository has been archived by the owner on Apr 28, 2022. It is now read-only.

separate out provider logic into files #18

Closed
wants to merge 1 commit into from
Closed

separate out provider logic into files #18

wants to merge 1 commit into from

Conversation

defunctzombie
Copy link

The files are put back together with "make dist". The result is easier to maintain code and a smaller minified version. Changed to use google closure api to minify vs uglifyjs. Uglifyjs can still be used and does produce a smaller version than before with the new dist/analytics.js file.

The providers.html test passes. The analytics.html test doesn't pass and it was not clear to me why. I believe it can be made to pass very easily.

I realize this is a lot of changes, but I hope you guys think about how it affects code clarity. Anyhow, even if you don't take the changes, cool project ;)

  • Changes can be tracked easier on a per provider basis.
  • fix trailing comma with intercom
  • update makefile to use google closure compiler to build
  • built files go into dist folder
  • update tests to use built version

* Changes can be tracked easier on a per provider basis.
* fix trailing comma with intercom
* update makefile to use google closure compiler to build
* built files go into dist folder
* update tests to use built version
@ianstormtaylor
Copy link
Contributor

Wow this is awesome, splitting into multiple files was the next thing on our todo list! I'll look over this early next week and if everything goes well, merge it in

Thanks!

@calvinfo
Copy link
Contributor

Yeah, this looks very cool. We'd been kicking around the idea of splitting up the files so the project is generally easier to deal with. It also opens up the opportunity for us to bundle only the providers that you end up using.

My main concern is that reunion is really young, and we'd require people to rely on it when building a new distribution version. Our current plan is to try and keep our library as dependency-free as possible.

Since we're just doing a client-side lib, we're going to try the modernizr approach of adding the providers by concatenating files. The nice thing is that it requires pretty much just OS-level functions, and seems to be working well for them. Using the hosted google closure compiler over uglify seems like a good addition though.

Coming from node, I totally agree that the commonjs format is nicer to work with than AMD. I think the argument for a tool which uses commonjs with less overhead than browserify is actually a really good one. Node libs which work on the client-side either seem to do their own hacky version of file re-writing or put everything in a single file. Reunion does seem like a nice improvement in the general case.

I'll probably use it for my next hack-project, when I'm building a site rather than a library. I'm excited to see you keep developing it! :)

@defunctzombie
Copy link
Author

@calvinfo The argument I would make for using a tool like reunion (which is the little sibling of the larger tool script which I use more heavily). The benefit of reunion is the very low overhead and no package.json/node_modules features. It was designed to be the lowest barrier to entry tool.

I would say that the biggest advantage of the commonjs approach (beyond being very easy to read) is that it will work instantly with tools like script/browserify/etc when installed as a dependency in an existing project using npm (or such). This is how I deploy and manage many of the dependencies for my web projects and since these tools can integrate right into a build step or middleware enchilada, the modules can be used as is. Anyhow, just wanted to point out some of the cool use cases you can get if you have the simple require system. All of the other stuff (single dist file, other loader support, etc) can all be done on top of the commonjs requires very easily.

The concatenation approach doesn't lend itself well to use within other modules already using commonjs and relies on making sure you keep the concatenation part of the makefile updated with any files you want in the right other and is more error prone imho. I know dependencies can be a pain, but I can tell you that the goal of reunion is to be super simple and do only one thing: build a small client side focused project for those not using these tools already.

@ianstormtaylor
Copy link
Contributor

fixed by 7423f8c

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants