Skip to content

Conversation

@Raynos
Copy link

@Raynos Raynos commented Apr 19, 2014

I broke out the DefaultDOMPropertyConfig.js file into a module called dom-property-config

  • Added a thorough README for dom-property-config
  • Added package.json boilerplate
  • Added a grunt task as a generic function for building a module
  • Updated the Gruntfile.js

The biggest value add in this change is

  • A really useful small part of React is available as a standalone module
  • Purely build based, no changes to development approach for React authors
  • A hidden internal now has a high quality piece of documentation.

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

@Raynos
Copy link
Author

Raynos commented Apr 21, 2014

I think tests fail because master is broken. I branched off of master.

Raynos added 6 commits April 21, 2014 13:29
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.
@Raynos
Copy link
Author

Raynos commented Apr 21, 2014

Just rebased this on latest master.

@Raynos
Copy link
Author

Raynos commented Apr 21, 2014

build passes now because i rebased on the "get master tests passing" fix from @zpao

@Raynos
Copy link
Author

Raynos commented Apr 22, 2014

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

Copy link
Member

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).

Copy link
Author

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')

@zpao
Copy link
Member

zpao commented Apr 23, 2014

And then when you get a chance, we'll need the CLA signed before we can pull this in (https://code.facebook.com/cla)

@Raynos
Copy link
Author

Raynos commented Apr 23, 2014

Signed the CLA.

@Raynos Raynos mentioned this pull request Apr 24, 2014
14 tasks
@zpao
Copy link
Member

zpao commented Jun 24, 2014

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 zpao closed this Jun 24, 2014
@Raynos
Copy link
Author

Raynos commented Jun 24, 2014

@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.

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.

3 participants