-
Notifications
You must be signed in to change notification settings - Fork 50.4k
Module for dom property config #1426
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
Conversation
|
I think tests fail because master is broken. I branched off of master. |
This prepares the DefaultDOMPropertyConfig.js file to be broken out by breaing it's tight coupling to DOMProperty.js This will reduce complexity and size of `dom-property-config` module
This will allow users of this module to understand what the flags mean without needing to know about the flags file
Must set the correct header including the providesModule directive
Written a README.md & package.json for the new module
Added a grunt target to build the dom-property-config module.
|
Just rebased this on latest master. |
|
build passes now because i rebased on the "get master tests passing" fix from @zpao |
|
Here is an example where the information that would be shared in this module is of great value to other framework authors ( MithrilJS/mithril.js#58 (comment) ) Mithril is another virtual DOM implementation and the author did not know about this edge case. Having this unit broken out as a shared piece of code allows the community to share the hard learned production lessons for virtual DOM systems. cc @lhorie |
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.
What's the plan when something like #1009 happens? Would we just merge the two here? Or would we want to export html & svg separately? (note, there will inevitably be some overlap).
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.
I would recommend that dom-property-config contains two files html.js and svg.js
Then you can require('dom-property-config/html') and require('dom-property-config/svg')
|
And then when you get a chance, we'll need the CLA signed before we can pull this in (https://code.facebook.com/cla) |
|
Signed the CLA. |
|
I'm going to close this, it's been sitting here a while without updates and I'm less sure we want this. Perhaps we can revisit later. |
|
@zpao no worries. I got side tracked on breaking this out. The conversion of dom properties to attributes & visa versa is still a difficult problem that I want to tackle but this might not be the right solution. |
I broke out the
DefaultDOMPropertyConfig.jsfile into a module calleddom-property-configdom-property-configThe biggest value add in this change is
This may be a good approach for other React internals. The biggest value of this approach is the ability to write a high quality targeted README.md for an internal piece of the React engine.
cc @petehunt @zpao