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.

Sign up for GitHub

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

Jump to bottom

Better support for unserializable props in fixture #15

Closed
wants to merge 13 commits into from
Closed

Better support for unserializable props in fixture #15

wants to merge 13 commits into from

Conversation

ovidiuch
Copy link
Owner

@ovidiuch ovidiuch commented Jun 1, 2015

  • state.fixtureUnserializableProps
    • compute on statics.getFixtureState, faster than on every editor change (onFixtureState)
    • deepEqual(prop, parse(stringify(prop)) to determine unserializable props Custom isSerializable helper
  • new state tests

@ovidiuch
Copy link
Owner Author

ovidiuch commented Jun 1, 2015

@w0rm can you please take a look?

It's a generic fix for react-cosmos/react-cosmos#130 and other use cases when fixture props aren't serializable (state should always be).

This is how I test Cosmos with a WIP ComponentPlayground branch: #14 (comment)

I'm not very happy with the ad-hoc isSerializable implementation. Also started a question on SO in the meantime: http://stackoverflow.com/questions/30579940/reliable-way-to-check-if-objects-is-serializable-in-javascript

@w0rm
Copy link

w0rm commented Jun 1, 2015

Looks fine to me. However I'm on vacations now and not able to try this out on a working project.

@ovidiuch
Copy link
Owner Author

ovidiuch commented Jun 2, 2015

Looks fine to me. However I'm on vacations now and not able to try this out on a working project.

No worries. Do you know when you'll be able to try it out? I can wait to make sure it covers your use case.

Enjoy your vacation!

@w0rm
Copy link

w0rm commented Jun 2, 2015

My vacation is until the 9th of June.

On 02 Jun 2015, at 12:40, Ovidiu Cherecheș notifications@github.com wrote:

Looks fine to me. However I'm on vacations now and not able to try this out on a working project.

No worries. Do you know when you'll be able to try it out? I can wait to make sure it covers your use case.

Enjoy your vacation!

—
Reply to this email directly or view it on GitHub.

@ovidiuch
Merge branch 'master' into 15-unserializable-props
225c86a 
Conflicts:
tests/components/component-playground/children.js
tests/components/component-playground/events.js
tests/components/component-playground/state.js
@ovidiuch
Rewrite unserializable props-related tests for new structure #15
2bece25 
Surfaces the need for
ovidiuch/react-component-tree#7
@ovidiuch
Unserializable props should be ignored from fixture editor contents #15
749ceca 
Old tests missed this
@ovidiuch
Merge branch 'master' into 15-unserializable-props
2ec0909 
Conflicts:
	tests/components/component-playground/children.js
	tests/components/component-playground/events.js
	tests/components/component-playground/state.js
@ovidiuch
Fake components yield weird behavior #15
ed2359a
ovidiuch added a commit that referenced this pull request Jun 7, 2015
@ovidiuch
Group and refactor tests #15
020b4f8
ovidiuch added a commit that referenced this pull request Jun 7, 2015
@ovidiuch
No longer deep clone fixture in every test #15
97e0401 
Was destroying unserializable members
ovidiuch added a commit that referenced this pull request Jun 7, 2015
@ovidiuch
Add support for unserializable props in fixtures #15
d3ba172
@ovidiuch
Copy link
Owner Author

ovidiuch commented Jun 7, 2015

Figured out some things were wrong on this branch so I started a brand new PR: #18

@ovidiuch ovidiuch closed this Jun 7, 2015
@ovidiuch ovidiuch deleted the 15-unserializable-props branch June 7, 2015 20:45
ovidiuch added a commit that referenced this pull request Jun 10, 2015
@ovidiuch
Bump package #15
71844f7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@ovidiuch @w0rm