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

add parse option to fetcher #385

Merged
merged 3 commits into from
Aug 1, 2014
Merged

Conversation

mdimas
Copy link
Contributor

@mdimas mdimas commented Aug 1, 2014

Adding this option ensures that the parse method is called on models whether they are loaded server or client side.

This solves simple issues like parsing a string into a Date, which would work on the first page load but then blow up during a client-side redirect when you try to use an attribute that should be a Date but is still a string.

I believe it also obviates the need for PR #372 and #380. Instead of recursively going through models with attributes that were parsed into models when constructing the initial bootstap data, they can be sent to the client as simple objects, therefore saving some overhead. Then when they're loaded client-side parse will be called, the sub-models will get loaded as models and automatically added to the cache.

@saponifi3d @mrubens

…recursive"

This reverts commit 6497522, reversing
changes made to 4dd5051.
…tions, this will ensure that parse is always called, whether it is loaded server or client side
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) when pulling d6bd6d7 on change:md_client_side_parse into ad38fe2 on rendrjs:master.

@saponifi3d
Copy link
Contributor

This is way better! 👍 now we aren't practically doubling the size of the bootStrappedData object.

saponifi3d added a commit that referenced this pull request Aug 1, 2014
@saponifi3d saponifi3d merged commit cd83109 into rendrjs:master Aug 1, 2014
@mdimas mdimas deleted the md_client_side_parse branch October 15, 2014 23:52
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

Successfully merging this pull request may close these issues.

3 participants