MeteorCommunity / discussions

Track technical discussions in the Meteor community
89 stars 7 forks source link

Official Meteor integration directly from 3rd party libraries #14

Open dandv opened 9 years ago

dandv commented 9 years ago

If you're looking for a popular 3rd party library on Atmosphere, chances are you'll find several wrappers, in various states of maintenance. To avoid this problem, several Meteor community members have started to provide authoritative, high-quality, Meteor integrations of 3rd party libraries, complete with tests and grunt scripts. These are forks of the library rather than Git submodules, because forks allow modifying the source to support Meteor (if necessary) and one can still pull upstream changes without having to re-apply the integration changes.

These forks live in the MeteorPackaging GitHub organization. This org was created so that multiple developers can collaborate on maintaining an integration in a centralized location. Now that Meteor's Package API is stable, we can submit PRs to popular JS library maintainers to add official Meteor integration. Some maintainers will accept the PR - great, we have integration straight from the source, with same-day updates on Atmosphere. Some won't accept the PR, in which case we continue with the MeteorPackaging fork.

I started this effort in November 2014, spoke about it at DevShop SF, and here's where we are now.

Going forward

As of September 2015, see http://autopublish.meteor.com. It's a system authored by @splendido that automatically publishes new versions of a wrapper package automatically on Atmosphere (once you've registered your repo with it), as soon as the upstream package has a new release. See the How it works page. For an example repo, see https://github.com/MeteorPackaging/numeraljs-core-wrapper.

The instructions from here on may be somewhat outdated.

Contribute (minimal effort)

  1. If you're working on integrating a 3rd party library with Meteor, please post here. I have also reserved Meteor Developer (Atmosphere) organization namespaces for many libraries; ask if you need one.
  2. If you have created an MD organization for a library, please let us know. Namespace is precious. Currently we're looking for:

    Who has these?

    Popular libraries with official Meteor support

    • :white_check_mark: d3 - [make, export file]
    • :white_check_mark: Bootstrap Material Design by @FezVrasta. Use the PR as a template.
    • :white_check_mark: Sortable [first merged!]
    • :white_check_mark: Keypress [merged 2014-11-28]
    • :white_check_mark: KeyboardJS [export file]
    • :white_check_mark: summernote - WYSIWYG editor; [2014-11-28]
    • :watch: Semantic UI [discussion in progress]

      PR acceptance pending

    • :clock1: Mousetrap - keyboard events library [pending] [grunt, export file]
    • :clock10: Font Awesome - [acknowledged]. Great example of Meteor resolving CSS URL paths correctly.
    • :ballot_box_with_check: Moment [merged in develop, published]
    • :clock1: Isotope [pending] [grunt, export file]
    • :clock930: jsPlumb [pending, maintainer was in a pissed off mood today]
    • :clock8: Hammer - multi-touch library, ~10k GitHub stars [pending, author has changed jobs] [grunt, export file]. Use it as a template.

      Refused to accept the PR

    • :no_entry_sign: keymaster
    • :no_entry_sign: Bootstrap - with much love and offering a Tweet

      How to do this

      Overview

  3. Post here asking @dandv and @splendido to add you to the MeteorPackaging organization and mention the library/libraries you want to package.
  4. We'll fork the library's repo into MeteorPackacing and grant you access
  5. Add the Meteor integration files,
    • package.js
    • tests
    • gruntfile (or makefile or equivalent) entries to run meteor publish
  6. publish the package to Atmosphere yourself (this is very important)
  7. submit a PR with the changes to the original library author (optional), or just notify them to get some exposure (they may offer to tweet about the integration or otherwise announce it)

Below are the detailed steps.

Get the integration working before even contacting the maintainer

  1. Fork
  2. git clone <your_fork_url>
  3. cd <repo_directory>
  4. git remote add upstream <repo_url_of_the_library>
  5. git fetch upstream
  6. Read through CONTRIBUTING.md or README.md and figure out which branch the original author wants pull requests against. Some can be very finicky.
  7. git checkout -b meteor-integration upstream/that_branch
  8. Create a meteor directory in the root of the project and copy there the Hammer.js Meteor package files, or the Font Awesome Meteor package files, depending on whether you integrate a library or CSS/image/font resources.
  9. Go through every file and make changes to reflect the actual library you're working on.

    • in particular, give a GOOD summary in package.js (for SEO on Atmosphere), instead of the library's tagline. Example:

      Mousetrap is a simple library for handling keyboard shortcuts in Javascript.

      For Atmosphere, a repository of JavaScript libraries, this is suboptimal: it repeats the library name, and it says "library" and "JavaScript" (well duh). You only have 100 characters. Don't waste them. Better:

      "Mousetrap (official): bind, trigger and handle keyboard events on keys, combinations and sequences"

  10. Export the library global(s)

    1. If the package is a jQuery package and adds itself to $, there's nothing to export.
    2. Otherwise, add a meteor/export.js file like this:

      /*global Hammer:true*/  // Meteor creates a file-scope global for exporting. This comment prevents a potential JSHint warning.
      Hammer = window.Hammer;
      delete window.Hammer;
  11. Add a package test file, at least checking if the library instantiates correctly.
  12. You want to make it as painless as possible for the maintainer to accept the PR. If there's a build process they use to publish, it would be great if you could add to it. Here's a gruntfile patch for Meteor and a Makefile patch (used for d3). If their README mentions Component/Bower/Npm, add a section mentioning Meteor (example).
  13. Run the tests using the build process, e.g. grunt meteor-test. If you want to run tests manually, execute this:

    cp meteor/package.js .
    spacejam --mongo-url mongodb:// test-packages ./

    Pick a namespace and publish the package under it

Please be very careful with this, as renaming packages is impossible.

Log into your Meteor Developer account and create an organization that represents the package. Most often it will be the name of library, without any special characters (no -, .). No need to include the JS part, since we're always dealing with JavaScript libraries.

If the organization name is not available, contact me, @dandv. I've registered a bunch of them, so potentially evil individuals don't :)

  1. meteor publish the package to Atmosphere for the first time. This prevents the maintainer from running into an error that the package doesn't exist when they publish (meteor would complain about a --create flag).
  2. After you've ensured publishing works (namespace is available, the name in package.js is valid etc.), push to your git fork:

    git commit
    git push origin meteor-integration
  3. Congratulations! Reply to this issue and add the link to the package's Atmosphere page.

Now you can go to the GitHub page of the library and issue a pull request!

Contact the library maintainers

Here is the recommended PR message.

See d3 and Font Awesome above for addressing objections.

Contact the maintainers of other Meteor wrapper libraries

  1. Ask them to deprecate the library by creating a PR overwriting the README.md of their package with the text

    This package is now deprecated. Please use the official Meteor integration package:
    
    http://atmospherejs.com/<org>/<package>

    The commit message should be "Deprecating package in favor of official integration".

    The name of the PR issue (after you click "Create pull request) should be "Kind request to deprecate package in favor of official integration".

    The text of the PR should be similar to this:

    Hi X,
    
    Thanks for publishing this package. In the meantime an
    [official Meteor integration](https://atmospherejs.com/<namespace>/<package>)
    has become available.
    
    Would you like to support the official package?
    
    If so, you can hide [your package](<atmosphere_link_to_their_package>)
    from Atmosphere searches, while still keeping it installable by apps and dependent
    packages, by running the following command:
    
       meteor admin set-unmigrated <their_full:package_name>
    
    Thank you,
    The [Meteor official integations team](https://github.com/raix/Meteor-community-discussions/issues/14)
  2. Go to the Atmosphere page of their package and flag it anyway. Ideally, get others to flag the wrapper package. This is important because meteor doesn't warn the user when running meteor add on a deprecated package, so we hope users will check on Atmosphere. However, each package needs several flags from different users, in order to show as flagged globally.

    TODO

    • figure out namespacing exactly. Ideally we'd have an MD organization with the library name, and ask the library maintainers to create MD accounts reflecting their GitHub usernames. Problem:
    • sometimes the names are taken, e.g. moment (pinging @estark37)
    • sometimes the maintainer uses their name as the organization, and you can't have an org and individual with the same name on MD - e.g. RubaXa/Sortable
    • rarely, several libraries have the same name, e.g. RubaXa/Sortable and voidberg/html5sortable (contrived example, need a better one)
splendido commented 9 years ago

@HedCET, clever approach indeed!

It obviously has the advantages you mentioned but, if I may, from a pure Meteoric point of view loading from CDN has also some cons:

  1. it adds more latency, especially if you're using more than one library this way
  2. it doesn't allow you to control the load order of the library you're using (and I'd expect some meteor code to fail trying to use something you're still loading from the CDN...)
  3. you can't build packages depending on CDN loads (?)

While, using proper meteor packages, the other way around for each of the above points is:

  1. allows to minify al js and css files alltogether (letting possible updates go through HCR) which will be loaded from your server with one single request
  2. as soon as you'r app starts all the code it needs is already available
  3. packages' dependecies are straightforward (?)

Does this makes any sense?

splendido commented 9 years ago

@raix, it seems that with the Meteor login there's little to do as per the official docs Do you have any different info? ...maybe with the user token you can build up a valid .meteorsession file? Have you already tried this out?

...differently the only way to do a meteor login on the build machine would be directly asking for the user password which is not the best thing to do... :(

splendido commented 9 years ago

@dandv: just created a ramda org and added you and @kevohagan. ...another addition for your reservations list ;-)

Kevin is probably going to try get his kevohagan:ramda officially published for Meteor next week

splendido commented 9 years ago

...some more random thoughts:

In case for some library we'll not get the meteor integration PR accepted, we might think about asking the maintainers to simply accept to have a new web hook on their repo (to be activated on autopublish.meteor.com). Then keep a fork of the library repo on MeteorPackaging and get autopublish.meteor.com sinchronize the forked repo with the mainstream one and meteor publishing at every hook trigger... We might think at some standard script/task/whatever to be run on the repo to update the package.js file to get the version number updated (to be run by autopublish.meteor.com), but this is all we'd need to get a new in (real) time publish at every new release.

I think this should be even more acceptable by repo maintainers, just in case they wouldn't even accept to have a new package.js file among their files...

What do you think? @dandv?

mizzao commented 9 years ago

@HedCET and @splendido a hybrid way is to have the server load the assets from CDN and serve them in the bundle along with the rest of the JS; this allows for flexibility of loading different versions but still controlling file load order and latency.

My https://github.com/mizzao/meteor-build-fetcher is one attempt to do this.

splendido commented 9 years ago

Good work @mizzao!

...it's interesting to see how different devs came out with their own different solution. each one in its own clever way :)

Perhaps the problem with this one is that you'd need to find out all needed files by hands and list them one by one ...and meteor won't warn you when a new version is available?

...I know this is being fussy ;-)

mizzao commented 9 years ago

@splendido I agree about the listing files problem, but a lot of libraries served over CDN are just one or two files. Also, one often bumps versions intentionally (and reviews the changelog).

Anyway, my original use for that was to not have to store libraries inside git repos. But I just wanted to mention that it may be possibly useful for the case you outlined above. Adding a library to an already minified bundle only marginally increases its size, versus separately downloading a new file from a different server.

dandv commented 9 years ago

@dandv: just created a ramda org and added you and @kevohagan. Kevin is probably going to try get his kevohagan:ramda officially published for Meteor next week

Great! I've edited https://atmospherejs.com/ramda and @kevohagan will probably want to publish to ramda:ramda?

dandv commented 9 years ago

@mquandalle: the steps are indeed a bit detailed, so it was easy to miss "Here is the recommended PR message." above. Gotta trim them down, but after @splendido gets autopublish fully working, the whole point might be moot.

dandv commented 9 years ago

Adding a library to an already minified bundle only marginally increases its size, versus separately downloading a new file from a different server.

If all libraries come from the same CDN, the browser might also have limits on the number of parallel requests, which would slow downloading all deps. A bundle is just one file.

hedcet commented 9 years ago

@splendido

i think this is also an advantages of CDN https://github.com/meteor/meteor/issues/3454#issuecomment-69463592

splendido commented 9 years ago

@HedCET you're right, sometimes there are some tricky passages to get an external library integrated as a meteor package... globals are major problem occasionally.

btw, I suspect you're not going to get an answer on the above issue: MDG usually redirects this kind of questions to meteor-talk (with quite a rude way sometimes... ;-), n° 3215 is among the last closed this way... )

raix commented 9 years ago

Just restore old global after lib code

splendido commented 9 years ago

ok guys, http://autopublish.meteor.com is ready for testing :)

I've also tried to add some docs here ...you can also have a look at the video in there ;-)

For those interested in the project, please continue the discussion in its issue tracker There are some important aspects to sort out...

Have a nice evening, Luca

tomByrer commented 9 years ago

If being loaded via CDN vs originating host, there will be about the same number of HTTP1.1 connections, sometimes more: http://tombyrer.github.io/slideshow-improving_load_times_with_cdns/#Browsers_HTTP_Connections Most likely less latency over all.

Of note: jsDelivr.com is really only for consumers, so we try to host only minified files (though a few are grandfathered in, & IIRC we do host .map files). One advantage of jsDelivr is server-side collation: https://github.com/jsdelivr/jsdelivr/blob/master/README.md#url-structure (Aside from the 0-downtime from the CDN multiplexing.)

We have a bot that can be set up to upload files from Github.com. @jimaek has set up other mirrors, if you want to mirror Meteor packages, & ask about dev file hosting. On Jan 9, 2015 5:33 AM, "Luca Mussi" notifications@github.com wrote:

@HedCET https://github.com/HedCET, clever approach indeed!

It obviously has the advantages you mentioned but, if I may, from a pure Meteoric point of view loading from CDN has also some cons:

  1. it adds more latency, especially if you're using more than one library this way
  2. it doesn't allow you to control the load order of the library you're using (and I'd expect some meteor code to fail trying to use something you're still loading from the CDN...)
  3. you can't build packages depending on CDN loads (?)

While, using proper meteor packages, the other way around for each of the above points is:

  1. allows to minify al js and css files alltogether (letting possible updates go through HCR) which will be loaded from your server with one single request
  2. as soon as you'r app starts all the code it needs is already available
  3. packages' dependecies are straightforward (?)

Does this makes any sense?

— Reply to this email directly or view it on GitHub https://github.com/MeteorCommunity/discussions/issues/14#issuecomment-69328777 .

hedcet commented 9 years ago

@tomByrer thank u for slides, jsdelivr:cdnify is the same path and much more

tomByrer commented 9 years ago

Welcome!

jsdelivr:cdnify is the same path and much more

Can you please substantiate this more please? Seems CDNfy has nice dashboards (so do we but they're not public yet), but I think jsDelvr's 69 POPs vs CDNify's 17 can't compare. The DNS network we use is bigger.

hedcet commented 9 years ago

jsdelivr:cdnify is a simple meteor package https://atmospherejs.com/jsdelivr/cdnify,

splendido commented 9 years ago

Ladies and Gentlements, what time is it? It's autopublish.meteor.com time! :)

The first meteor integration PR exploiting autopublish for Twix.js has been succesfully completed! Thanks @icambron! :-)

twix_autopublish

...and we also got from MDG the assurance to have no down times for autopublish.meteor.com

I think this is a very good starting point! ...though there's still a lot of work to be done ;-)

splendido commented 9 years ago

Please see and star momentjs:twix

dandv commented 9 years ago

@splendido: Splendid work! I need to catch up with autopublish.

In the meantime a Twitter user has requested Chart.js so I just wrapped it manually (hopefully the last manual wrapper!).

Please vote for https://atmospherejs.com/chart/chart and flag:

splendido commented 9 years ago

:+1:

rgoomar commented 9 years ago

@splendido nice work on the auto publisher! I'm going to have to setup my repos to start using that.

splendido commented 9 years ago

ok @rgoomar, lets see what happens. We'll wait for your feedback ;-)

rgoomar commented 9 years ago

@dandv Another one to add to the list? https://github.com/formvalidation/formvalidation

ccorcos commented 9 years ago

Hello, I'd like to add lodash to this. Can I get permission for MeteorPackaging org and maybe a little help.

rgoomar commented 9 years ago

@splendido The autopublish is working! The only thing I didn't realize is that the hook is based on tags / releases. :)

splendido commented 9 years ago

@ccorcos: I've just added you to the Packagers team. But I'm not sure you can fork in there. In case you cannot, let me know what repo you would like to fork.

@rgoomar great! Tnx for testing it out :) ...btw, haven't you read how it works?! if you push some steps, it's clearly said that the webhook is triggered on new tags ;-) Kidding apart, I'm open to discussion, but I though it was the only reasonable solution. Also consider that in case you push a new Release a new tag is also created so actually we can pick-up new tags and new releases. What do you think about this? ...would you like to publish any new commit or what?

mrjf commented 9 years ago

@dandv I'm creating a package for proj4js: https://github.com/proj4js/proj4js

Could I get permission to fork into MeteorPackaging org, or failing that, could you fork the above repo in there for me? Thank you.

dandv commented 9 years ago

@mrjf Done! Welcome to the Meteor Packagers team.

mrjf commented 9 years ago

Thank you! Fork created ( https://github.com/MeteorPackaging/proj4js ), will push to it soon.

splendido commented 9 years ago

I should create a list of PRs presented so far for autopublish.meteor.com so that anyone willing to create new ones can take a look at some reference example.

I'll be back to you tomorrow @mrjf!

mrjf commented 9 years ago

@dandv @splendido I've committed the necessary bits to update the library for meteor packaging:

https://github.com/MeteorPackaging/proj4js/commit/82f0c98dd314c314d57f81e324fe953f692e3665

I used the moment and hammer changes as models. I published it ( https://atmospherejs.com/proj4/proj4 ) and used the package successfully.

If you have a chance, I'd love to get your expert eyes on my commit before I send it as a PR to the proj4 maintainers. In fact, if you'd prefer to initiate the PR more officially, that'd be fine with me too.

Thanks for the thorough instructions in this document. Couple things to note:

splendido commented 9 years ago

Hei @mrjf, I think we're now more oriented about leveraging autopublish.meteor.com to prevent asking people to run scripts and commands on new releases.

Please have a look at the latest PRs referenced above here (5 posts up, more or less...)

splendido commented 9 years ago

Ok guys, lets have a look at the Wiki I've started on the autopoblish.meteor.com repo.

Feel free to add/modify texts and pages. Hopefully that would be a better place where to keep things ordered.

Let me know what do you think, Luca

dandv commented 9 years ago

@mrjf, thanks for your work! @splendido has added your caveats to the wiki and I've just finished proofreading it. LGTM.

splendido commented 9 years ago

:+1:

Urigo commented 9 years ago

Hi @dandv ,Great job on doing some very needed order in the package area!

I want to add an official ui-router package support for Meteor.

I've followed your instructions, forked ui-router and added the needed changes here: https://github.com/Urigo/ui-router/commit/cd10ce17bdbb08ad0367a654c19027ca069488b5

would love to have your review before submitting it!

I've also created a Meteor organization 'angularui' (until maybe 'angular-ui' will be possible - https://github.com/meteor/meteor/issues/3658) and published the package under this name: angularui:angular-ui-router

Should I add you to that organization?

Before I send the pull request, will it be better to do it under MeteorPackaging Github organization (I don't mind copying the changes to a new fork)?

By the way, my next PR would be for Angular library itself but the 'angular' organization is not available...

Thanks

ccorcos commented 9 years ago

@splendido no permission to fork lodash to this group...

splendido commented 9 years ago

@ccorcos gimme the link and I'll do the fork....

ccorcos commented 9 years ago

https://github.com/lodash/lodash

splendido commented 9 years ago

@ccorcos: forked!

ccorcos commented 9 years ago

@splendido so what exactly do I need to do here? Seems like I can just add a package.js that loads lodash.js to the client and the server and we're good to go. Just need to keep pulling in commits from lodash whenever they happen. Anything I'm missing?

splendido commented 9 years ago

@ccorcos, lets have a look at the Wiki I've started on the autopoblish.meteor.com repo.

For new releases autopublish.meteor.com will do the rest ;-)

mitar commented 9 years ago

One suggestion: https://github.com/meteor/meteor/issues/3681

dandv commented 9 years ago

@Urigo: thanks for your work! I left an inline comment in the PR.

Before I send the pull request, will it be better to do it under MeteorPackaging Github organization

This was actually a bit of a problem with the listed steps in the OP, because someone had to ask for forking access to the MeteorPackaging org.

Let's try something else - can you transfer your repo to MeteorPackaging? If so, let's open an issue there and continue this thread.

Should I add you to that organization?

Yes please. I've just added you to angularjs, but I don't know who has angular, and it's MDG's policy to not take away MD org names from squatters.

Urigo commented 9 years ago

@dandv

Next, about AngularJS, their build process is more complex. but they do publish every release to designated bower repositories like that: https://github.com/angular/bower-angular https://github.com/angular/bower-angular-animate

Maybe as a first step we can listen on that and auto-publish a Meteor package?

On Thursday I'm going to the AngularJS meetup in mountain view and might be able to talk to someone from the team about adding the Meteor package into their build process.

ccorcos commented 9 years ago

I'm having an issue with integrating lodash. I have a package.js like this:

Package.describe({
  name: 'lodash:lodash',  // http://atmospherejs.com/lodash/lodash
  summary: 'A JavaScript utility library delivering consistency, modularity, performance, & extras.',
  version: '3.1.0',
  git: 'https://github.com/lodash/lodash.git'
});

Package.onUse(function (api) {
  api.versionsFrom('METEOR@1.0');
  api.addFiles([
    'lodash.js'
  ], ['client', 'server']);
  api.export("_", ['client', 'server']);
});

But it seems that underscore is overwriting lodash and I can't remove underscore because its not "underscore is not in this project."

Any thoughts?

raix commented 9 years ago

Make the lodash package depend on underscore - this will ensure that underscore is loaded first. I normally add two files surrounding global exports - one stores the original globals and the last restores them + sets the proper vars for Meteor to export.

  // A: store org scope
  org = {
    '_': window._
  };

  // B: Load underscore

  // C: Prepare export
  _ = window._;

  // D: Restore
  window._ = org._;
splendido commented 9 years ago

@raix if the aim is to propose an official integration with a PR to the upstream repository, I'm not sure it will be possible to add extra code... From the (few) experiences we recently had, proposing to add more files is not always a good idea...

@ccorcos: depending on how the library is initialized, it might be that you don't need api.export. Most of the times you'll find it already set up as a global var...