Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Detect content changed #225
Detect content changed #225
Changes from 2 commits
4658e88
c36e1c1
5180b0c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just an idea: we can make use of ES6 classes for State to make use of constructors, getter&setters, for example every time the html is set we can update blocks and initialHtmlHash inside the class. or we can define a constructor that is taking html as input and update blocks and initialHtmlHash in that constructor. again this is just an idea current version is also ok for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me see if I understand the proposal: Will that class update its internal vars?
If yes, I'm not sure that's what we want for Redux. My understanding is that the reducer is supposed to return a new "state" object and avoid mutating the old one, isn't it? In that case, I'm not sure a State class would be too useful since we would essentially only use its constructor, and that's pretty much what
html2State
performs.Let me know if there is something in the proposal that I'm missing, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, and btw, we might don't want to spend too much time iterating on this since we will be using GB's stores soon #226 anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right about mutating. we could figure it out though maybe there's an easy way to clone a class object. I'll investigate this further on a more free time and If I discover a useful practice I will post a p2 about it.