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

[do not merge] Allow optional converters #12

Closed
wants to merge 1 commit into from
Closed

[do not merge] Allow optional converters #12

wants to merge 1 commit into from

Conversation

pgoldrbx
Copy link
Contributor

@pgoldrbx pgoldrbx commented Dec 11, 2017

This is an initial PoC to allow for default pass-through conversion of nodes (see #10)
I'm making this available for early feedback.

Dependencies

Outstanding tasks

  • tests
  • update README documenationn

@pgoldrbx pgoldrbx self-assigned this Dec 11, 2017
@@ -3,6 +3,8 @@ import { validateConverters, visitNode } from './helpers';

const ERR_INVALID_XML = 'XMLToReact: Unable to parse invalid XML input. Please input valid XML.';

const noop = () => {};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

noop and throwError could move to helpers?

@@ -56,7 +60,6 @@ export default class XMLToReact {
return null;
}

return visitNode(tree.documentElement, 0, this.converters, data);
return visitNode(tree.documentElement, 0, this.converters, data, this.onConverterNotFound);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

5 params... i don't love it

@@ -82,7 +82,7 @@ export function getChildren(node) {
* @return {Object} React element
* @private
*/
export function visitNode(node, index, converters, data) {
export function visitNode(node, index, converters, data, onConverterNotFound) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think i'll change the function signature to something like (node, {/*the rest*/}) but i can try that on master

@pgoldrbx
Copy link
Contributor Author

pgoldrbx commented Sep 6, 2019

closing as abandoned

@pgoldrbx pgoldrbx closed this Sep 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant