ToothlessGear / node-gcm

A NodeJS wrapper library port to send data to Android devices via Google Cloud Messaging
https://github.com/ToothlessGear/node-gcm
Other
1.3k stars 206 forks source link

Make `addData(WithObject)` *add* data (not overwrite) #75

Open hypesystem opened 9 years ago

hypesystem commented 9 years ago

It seems misleading that the reference to the data object is overwritten with whatever is given to addDataWithObject.

A more sensical way to do it, would be to merge the two objects, overwriting any values that were already present.

This would allow known values to be set at time of construction, and later values to be added after:

var message = new gcm.Message({
  data: {
    knownValue: "atConstructionTime",
    otherValue: "thatMayChangeBeforeSending"
  }
});
//State: { knownValue: "atConstructionTime", otherValue: "thatMayChangeBeforeSending" }

message.addData({
  newValue: "thatIsOnlyAddedLater"
});
//State: { knownValue: "atConstructionTime", otherValue: "thatMayChangeBeforeSending", newValue: "thatIsOnlyAddedLater" }

if(something) {
  message.addData({
    otherValue: "thatHasNowBeenChanged"
  });
  //State: { knownValue: "atConstructionTime", otherValue: "thatHasNowBeenChanged", newValue: "thatIsOnlyAddedLater" }
}
hypesystem commented 9 years ago

This would be a breaking change... maybe we should add a branch for breaking changes that will be merged into master at 1.0?

hypesystem commented 9 years ago

it would make sense if there was also a set function doing pretty much what we do now, but for any value:

//Overwrite data field
message.set("data", {
    knownValue: "atSomePoint",
    otherValue: "theseOverwriteEverythingPreviouslySet"
});

//Set new value for one of the parameters sent to gcm
message.set("collapseKey", "newCollapseKey");
hypesystem commented 9 years ago

If data gets its own addData method, notification should have the same:

message.addNotificationField("title", "Hello, World");