Skip to content
This repository has been archived by the owner on Nov 10, 2017. It is now read-only.

Add a DaybedStorage PoC #25

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Add a DaybedStorage PoC #25

wants to merge 6 commits into from

Conversation

Natim
Copy link
Collaborator

@Natim Natim commented Aug 4, 2014

Lots of things to change before it works:

  • Add daybed.js and hawk.js to vendors
  • DaybedStorage is promise aware, I don't know how to get it integrated with react and getInitialState.

Hope @n1k0 can help me on this.

@Natim
Copy link
Collaborator Author

Natim commented Aug 4, 2014

I used componentDidMount and it worked :)

componentDidMount: function() {
this.props.store.load()
.then(function(items) {
if (this.isMounted()) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're in componentDidMount, component is necessarily mounted.

for (var i = 0; i < doc.records.length; i++) {
var record = doc.records[i];
record.type = type;
data.push(record);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit:

data = doc.records.map(function(record) {
  record.type = type;
  return record;
});

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well you don't want to override data at this point.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I can still use map 👍

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

data = data.concat then

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

race condition detected with data.concat

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well maybe not, let me try.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be very surprised if concat would where a for loop wouldn't.

try {
this._store["keep.data"] = JSON.stringify(data);
} catch (e) {
console.error("failed saving keep data", e);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

call reject() ?

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

Successfully merging this pull request may close these issues.

None yet

3 participants