Modified to add AMD registration #58

Closed
wants to merge 7 commits into
from

Conversation

Projects
None yet
2 participants
@nekman
Contributor

nekman commented Jan 5, 2013

Register Backbone.LocalStorage as a AMD module.

Example of usage:

  // Example configuration for require.js
  require.config({
    baseUrl:'js',
    paths: {
      jquery : 'https://ajax.googleapis.com/ajax/libs/jquery/1/jquery.min',      
      backbone : 'lib/backbone',
      underscore: 'lib/underscore',
      localstorage: 'lib/backbone.localStorage'
    },
    shim: {
      'backbone': {
        //These script dependencies should be loaded before loading
        //backbone.js
        deps: ['lib/underscore', 'jquery'],
        //Once loaded, use the global 'Backbone' as the
        //module value.
        exports: 'Backbone'
      },
      'localstorage': {
        //These script dependencies should be loaded before loading
        //backbone.localstorage.js
        deps: ['backbone'],
        //Once loaded, use the global 'Backbone.LocalStorage' as the
        //module value.
        exports: 'Backbone.LocalStorage'
      }
    }
  });

  define('someCollection', ['backbone', 'localstorage'], function(Backbone, LocalStorage) {
    return Backbone.Collection.extend({
      localStorage: new LocalStorage('SomeCollection') // Unique name within your app.
    });
  });

  require(['someCollection'], function(someCollection) {
    // use someCollection
  });

Build status: https://travis-ci.org/nekman/Backbone.localStorage

+ // RequireJS isn't being used. Assume underscore and backbone is loaded in <script> tags
+ factory(_, Backbone);
+ }
+}(function(_, Backbone) {

This comment has been minimized.

Show comment Hide comment
@jeromegn

jeromegn Jan 6, 2013

Owner

Interesting pattern! Concise, efficient.

@jeromegn

jeromegn Jan 6, 2013

Owner

Interesting pattern! Concise, efficient.

@jeromegn

View changes

backbone.localStorage.js
@@ -1,9 +1,17 @@
/**
- * Backbone localStorage Adapter
+ * Backbone localStorage Adapter (modified to add AMD registration)

This comment has been minimized.

Show comment Hide comment
@jeromegn

jeromegn Jan 6, 2013

Owner

Maybe just leave it as "Backbone localStorage Adapter"?

@jeromegn

jeromegn Jan 6, 2013

Owner

Maybe just leave it as "Backbone localStorage Adapter"?

This comment has been minimized.

Show comment Hide comment
@nekman

nekman Jan 6, 2013

Contributor

Yes, of course. That should be removed. Fixed!

@nekman

nekman Jan 6, 2013

Contributor

Yes, of course. That should be removed. Fixed!

@jeromegn

This comment has been minimized.

Show comment Hide comment
@jeromegn

jeromegn Jan 6, 2013

Owner

Good work.

I'm just thinking the description in the issue would be nice to put in the README.md file. Create a section like "AMD loader" (but I feel like that's a poor title) as a sub-section "Usage".

I think that would make it a "complete" feature :) It has a simple implementation, a test and now it would have documentation.

Owner

jeromegn commented Jan 6, 2013

Good work.

I'm just thinking the description in the issue would be nice to put in the README.md file. Create a section like "AMD loader" (but I feel like that's a poor title) as a sub-section "Usage".

I think that would make it a "complete" feature :) It has a simple implementation, a test and now it would have documentation.

@nekman

This comment has been minimized.

Show comment Hide comment
@nekman

nekman Jan 7, 2013

Contributor

Jerome: I need to fix another issue with my code that I didn't discovered until now...By mistake, I used the Backbone and Underscore AMD forks (https://github.com/amdjs/backbone) for my example. I have fixed this now, but I think that it is best to close this merge request until everything is fixed and re-committed. I will also update the README.md in my next merge request.

Contributor

nekman commented Jan 7, 2013

Jerome: I need to fix another issue with my code that I didn't discovered until now...By mistake, I used the Backbone and Underscore AMD forks (https://github.com/amdjs/backbone) for my example. I have fixed this now, but I think that it is best to close this merge request until everything is fixed and re-committed. I will also update the README.md in my next merge request.

@nekman nekman closed this Jan 7, 2013

@jeromegn

This comment has been minimized.

Show comment Hide comment
@jeromegn

jeromegn Jan 7, 2013

Owner

Alright.

Looking forward to the final changes.

Owner

jeromegn commented Jan 7, 2013

Alright.

Looking forward to the final changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment