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

Add a DaybedStorage PoC#25

Open
Natim wants to merge 6 commits into
masterfrom
daybed_storage
Open

Add a DaybedStorage PoC#25
Natim wants to merge 6 commits into
masterfrom
daybed_storage

Conversation

@Natim
Copy link
Copy Markdown
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
Copy Markdown
Collaborator Author

Natim commented Aug 4, 2014

I used componentDidMount and it worked :)

Comment thread src/js/components/KeptApp.js Outdated
Copy link
Copy Markdown
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.

Comment thread src/js/store/daybed.js Outdated
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

Comment thread src/js/store/index.js
Copy link
Copy Markdown

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.

3 participants