Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

node-squarespace-middleware doesn't support multiple sites. #9

Open
foleyatwork opened this issue Oct 12, 2015 · 5 comments
Open

node-squarespace-middleware doesn't support multiple sites. #9

foleyatwork opened this issue Oct 12, 2015 · 5 comments

Comments

@foleyatwork
Copy link

So this issue is a bit more complex than the last one I opened. I have this app that is attempting to loop through an array of sites and perform a few actions on each. I really want to use middleware because it has all the login logic, and I can fix this on my end in a variety of ways, but they're all kind of sub-optimal.

Here's a sample that might hammer home what I'm doing.

var sqsMiddleware = require('node-squarespace-middleware');
// I'm going to skip writing the code but I set my email, password, and sandbox mode here.

var sites = [ 'https://example-one.squarespace.com', https://example-two.squarespace.com' ];
sites.forEach(function(site) {
  sqsMiddleware.set('siteurl', site);
  sqsMiddleware.doLogin(function(error, headers) {
    // headers will always be for example-one.squarespace.com
  });
});

I'm brainstorming ways I could fix this without screwing up the existing API. @kitajchuk, If you've got any ideas let me know.

@kitajchuk
Copy link
Owner

I think the issue right now is the middleware operates as a sort of singleton. We can probably rewrite it to be instance based that way you can operate multiple middleware instances in one script.

As for the code setup you have here, it dictates that the middleware could operate on many sites as a singleton. This is hard because it needs to retain the correct headers and crumb for any given siteurl. That being said, the middleware could operate as more of a manager. In this way you would basically configure a dictionary of sites and operate against them by ID or something. Or maybe the url can be the unique identifier for a site in this instance.

Does that make sense? Maybe the API looks more like this where site data is url, email, password, sandbox etc...

sqsMiddleware.configure({
    foo: {... site data},
    bar: {... site data},
    baz: {... site data}
});

And then you could perform actions like this(ish). And you would always be getting headers for the site you performed the login for.

sqsMiddleware.doLogin( "foo", function ( error, headers ) {...} );

Equally, you would perform other method calls in a similar fashion, always by ID.

sqsMiddleware.getJson( "bar", "{collection}", {...query string}, function ( error, data ) {...} );

@foleyatwork
Copy link
Author

I was thinking maybe this could move to a class pattern using either the new constructor or return a function that will create a new instance of the class. The API could look like this.

var sqsMiddleware = require('node-squarespace-middleware');

var siteOne = sqsMiddleware();
siteOne.set('siteurl', 'https://site-one.squarespace.com');
siteOne.set('email', '[email protected]');
siteOne.doLogin(function(){});

var siteTwo = sqsMiddleware();
siteTwo.set('siteurl', 'https://site-two.squarespace.com');
siteTwo.set('email', '[email protected]');

The obvious problem with this is it's a breaking change, which I guess we could solve by just bumping the version number?

@kitajchuk
Copy link
Owner

Yeah. I would bump the version a full minor after a refactor like this :)

@foleyatwork
Copy link
Author

Cool. I'm going to get a version working for my local project then submit a PR. If you don't think the project necessarily needs to support multiple sites, feel free to just reject it. I can always just run a custom version for my project.

@kitajchuk
Copy link
Owner

Do it man. I think it would be a good refactor overall for this module 👍

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

No branches or pull requests

2 participants