Skip to content

Serialization / Deserialization support for backbone-parse-es6 #144

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

Closed
typhonrt opened this issue Dec 28, 2015 · 13 comments
Closed

Serialization / Deserialization support for backbone-parse-es6 #144

typhonrt opened this issue Dec 28, 2015 · 13 comments

Comments

@typhonrt
Copy link

I'm trying to solve cleanly in backbone-parse-es6 the scenario of setting another "Backbone.Model" which is backed by a ParseObject in another "Backbone.Model". Please see backbone-parse-es6 Issue #5.

It seems the cleanest way to accomplish this is to be able to replace the encode function such that when toJSON->encode reaches a "Backbone.Model" it encodes the backing ParseObject, but that isn't currently possible since ParseObject imports encode directly. A possible way of doing this is exposing encode in Parse.js and have ParseObject import Parse.js and access the encode function from there. This will allow me to replace the encode function with a Backbone.Model aware version.

Of course there is already _encode in Parse.js for legacy support, but it's not accessed by ParseObject.

There are more hacky / fragile ways of solving this issue perhaps, so I'd like to find out if this change can be considered for the JS Parse SDK.


In regard to creating new a new Backbone.Model subclass from fromJSON in ParseObject I should still be able to use ParseObject.registerSubclass with the Backbone.Model class and simply implement _finishFetch and _setExisted in the backbone-parse-es6 Backbone.Model class forwarding those method calls to the backing ParseObject.


As a sidenote I'm sure there will also be a similar situation when associating ParseRelation data, so I'll try and test these concerns such that if changes are to be made backbone-parse-es6 can support creating pointer and relation data when serializing / deserializing.

@typhonrt
Copy link
Author

@andrewimm
I'm getting close on a solution for this that is somewhat unobtrusive. It did require me to extend Parse.Object for a few tricky manipulations involving private methods. I'll post things here in a day or so; no need to answer any of the above. I would like to get some consensus / buy in on the fragility of my solution though with your future expectations for the Parse SDK.

@andrewimm
Copy link
Contributor

Sounds good, looking forward to it.

@typhonrt
Copy link
Author

typhonrt commented Jan 8, 2016

Well,
I've spent a good time on this backbone-parse-es6 issue. I have a solution, but it requires some modification of Parse-JS SDK as things go for the cleanest path. A quick recap. The problem is when adding a Backbone Model to another Model by default Parse doesn't know how to serialize the backing ParseObject which is stored in the parseObject variable.

Likewise with deserialization changes need to be made to create a proper Backbone.Model / ParseModel sub class.

The cleanest path to support serialization is to create a custom BackboneParseObject which overrides the following methods:
_getSaveJSON and toJSON

Both of these methods defer to the backing ParseObject if it exists.


Deserializing is more of a headache due to ParseObject->fromJSON being a static method which is accessed most importantly from ParseQuery and decode.

I need to be able to override fromJSON to handle things cleanly and here is an example change that doesn't do any type checking:

   static fromJSON(json)
   {
      if (!json.className)
      {
         throw new Error('Cannot create an object without a className');
      }

      // Ideally we would create a BackboneParseObject here.... 
      var parseObject = new ParseObject(json.className);

      var otherAttributes = {};
      for (var attr in json)
      {
         if (attr !== 'className' && attr !== '__type')
         {
            otherAttributes[attr] = json[attr];
         }
      }
      parseObject._finishFetch(otherAttributes);
      if (json.objectId)
      {
         parseObject._setExisted(true);
      }

      // constructor is assumed to be a Backbone.Model / ParseModel
      // in a properly overloaded version of this method some protection would be involved here. 

      var constructor = classMap[json.className];
      var o = constructor ? new constructor({}, { parseObject }) : parseObject;

      return o;
   }

The subtle difference above is always creating the ParseObject then if a sub class is registered that is a Backbone.Model / ParseModel then associate the ParseObject with it. As things go figuring out how to provide a way to import a modified fromJSON method perhaps from Parse.js seems like the direction to proceed.

Two additional proposed helpful static methods for ParseObject are getSubclass(className) which returns any registered sub class by name and hasSubclass(className) which returns a boolean if a class is registered since classMap is not accessible / module private.


Please advise @andrewimm All my other attempts to work around the static fromJSON method were really nasty / fragile / error prone. The above changes I mention are working great in a proof of concept implementation.

@typhonrt typhonrt changed the title Replaceable encode function in Parse.js for ParseObject.js Serialization / Deserialization support for backbone-parse-es6 Jan 8, 2016
@typhonrt
Copy link
Author

Hi @andrewimm. I'm just bumping this especially since the 1.7.0 release is on the horizon given a change like exposing a replaceable non-static fromJSON would fit the motif of a new major version release. Exposing fromJSON from Parse.js seems reasonable. It'll help any 3rd party binding given the extra flexibility.

Cheers...

@typhonrt
Copy link
Author

@andrewimm I'm bumping this issue again. Perhaps now that 1.7.0 is out we can take a look at improving 3rd party library support for deserialization. As mentioned above the best path forward is making fromJSON a non-static method perhaps accessible from Parse which will allow it to be easily replaced as necessary by 3rd party libraries like backbone-parse-es6.

Do let me know if there is any possibility of this. It's been a month now w/ this issue open. I've tested this solution and it solidly works. If necessary I'll even make a PR once I get your buy in to the solution.

Thanks...

@typhonrt
Copy link
Author

@andrewimm I'm still waiting on any response / acknowledgement to the solution I've outlined above. What happened to "Sounds good, looking forward to it."? I'd like to close the only bug in backbone-parse-es6 and this pertains to making the Parse JS SDK more friendly to third party integration. I spent quite a few hours testing out my solution. I'm not going to create a PR until you acknowledge the solution and will give it a look for acceptance. I'm not interested in wasting my time, so it'd be great if you can take a look and agree to something even if it is that you'll do nothing.

@AlexisBarta
Copy link

+1 in favor of @typhonrt I'd like to know if it's something that you'll think of or not.

@andrewimm
Copy link
Contributor

We've been busy with other SDK work, sorry for the silence on this. I'm still not clear on how you propose to expose fromJSON from Parse.js, because it's a property of Parse.Object. From an API perspective, fromJSON is a property of Parse.Object, and doesn't make sense living elsewhere. If you need to change the behavior, why can't you override the property directly on Parse.Object?

@andrewimm
Copy link
Contributor

I'd love to help where possible, but so far the proposal sounds a bit heavy-handed and tailored towards this singular use case.

@typhonrt
Copy link
Author

typhonrt commented Mar 3, 2016

As things go I guess I overlooked that one can replace a static method. So I have been successful in getting things to work without modification of the Parse JS SDK. The only side effect is that I also have to override registerSubclass as it accesses the module private classMap object in addition to providing a new getSubclass method.

To reduce the amount of custom code I need to override could you consider adding a static getSubclass(className) method which returns the value in classMap. It would be handy to add a convenience method hasSubclass(className) returning a boolean if one exists.

Should I file a new issue for addition of getSubclass / hasSubclass?

@typhonrt
Copy link
Author

typhonrt commented Jul 12, 2016

@andrewimm Well... I thought I came up with a workable solution, but it only works in JSPM or potentially browserify / webpack or other package managers that automatically pull in all dependencies (babel-runtime full NPM parse module w/ lib directory) though currently untested in the later two. The problem I'm having is the AMD / UMD bundles of backbone-parse-es6 being used with RequireJS directly. I've had a request to support RequireJS and as things go I didn't have a full test / canonical TODO demo in place previously, so wasn't aware of the problem until trying to support a legacy Parse web app via RequireJS.

The current workaround for serialization / deserialization that I figured out requires having access to the full encode method signature as seen here in BackboneParseObject. You can see the AMD bundle here which includes "parse/lib/browser/encode.js".

This causes problems with RequireJS due to including all the necessary dependencies and having to potentially provide the whole (or lots of dependencies) in the lib directory given the imports in encode.js. There is no clean workaround / hack for RequireJS consumption.

A simple solution is to expose the full encode method via setting Parse.encode = encode; in Parse. Might as well set Parse.decode = decode; as well in case the method signature ever changes from Parse._decode.

Thanks again for any consideration as I'm just trying to support legacy Parse apps that need to move to parse-server soon.

@typhonrt
Copy link
Author

@andrewimm any chance we can continue a discussion about my proposed solution above? I'd be glad to submit a PR once I know it will be accepted.

The crux of the solution is stated above: "A simple solution is to expose the full encode method via setting Parse.encode = encode; in Parse. Might as well set Parse.decode = decode; as well in case the method signature ever changes from Parse._decode".

@stale
Copy link

stale bot commented Feb 6, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Feb 6, 2019
@stale stale bot closed this as completed Feb 13, 2019
@dplewis dplewis added resolved and removed stale labels Oct 22, 2020
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

4 participants