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

Expose currentNode being patched #132

Merged
merged 2 commits into from
Oct 28, 2015
Merged

Conversation

markuskobler
Copy link
Contributor

While experimenting with idom I'm finding that while var el = elementOpen('div') works it can complicated code that wants to repatch elements later. Exposing the current Element makes the code a bit more readable.

Currently throws an error when not production build.

@@ -109,6 +110,58 @@ describe('element creation', () => {

});


describe('should return getCurrentNode', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems that this should probably be a top level describe (it its own file) since currentElement is really it's own feature that also applies on updates not just element creation.

@jridgewell
Copy link
Contributor

@markuskobler: Want to rebase and update based on the comments? I'm actually in need of this.

@markuskobler
Copy link
Contributor Author

Sorry for dragging feet on this, I have been distracted with other work the last few weeks.

I also added some fix's to the closure annotations which I hope are not too contentious. It is failing on idom/src/attributes.js still but I'm not quite sure how to safely fix the function (Element, string, *)

el = null;
});

it('from elementOpen', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

its should look something like:

it('should return the element from elementOpen', ...)

describes should describe the feature

describe('currentElement', ...

Ideally, the full thing would read as a sentence, for example in element_creation.js:

"element creation when creating a single node should render with the specified tag"

Also, context isn't really a feature as far as a consumer of the library cares. The feature they deal with is just currentElement.

@@ -61,14 +61,16 @@ var createNode = function(doc, nodeName, key, statics) {
if (nodeName === '#text') {
node = doc.createTextNode('');
} else {
node = createElement(doc, nodeName, key, statics);
node = createElement(doc, nodeName, key);
Copy link
Contributor

Choose a reason for hiding this comment

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

Whoops.

sparhami pushed a commit that referenced this pull request Oct 28, 2015
Expose currentNode being patched
@sparhami sparhami merged commit eb6b2ab into google:master Oct 28, 2015
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.

None yet

3 participants