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

Module for dom property config #1426

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions Gruntfile.js
Expand Up @@ -10,6 +10,7 @@ var sauceTunnelTask = require('./grunt/tasks/sauce-tunnel');
var npmTask = require('./grunt/tasks/npm');
var releaseTasks = require('./grunt/tasks/release');
var npmReactTasks = require('./grunt/tasks/npm-react');
var npmDomPropertyConfigTasks = require('./grunt/tasks/npm-dom-property-config');
var npmReactToolsTasks = require('./grunt/tasks/npm-react-tools');
var versionCheckTask = require('./grunt/tasks/version-check');

Expand Down Expand Up @@ -67,6 +68,10 @@ module.exports = function(grunt) {
grunt.registerTask('npm-react:release', npmReactTasks.buildRelease);
grunt.registerTask('npm-react:pack', npmReactTasks.packRelease);
grunt.registerTask('npm-react-tools:pack', npmReactToolsTasks.pack);
grunt.registerTask('npm-dom-property-config:release',
npmDomPropertyConfigTasks.buildRelease)
grunt.registerTask('npm-dom-property-config:pack',
npmDomPropertyConfigTasks.packRelease)

grunt.registerTask('version-check', versionCheckTask);

Expand Down Expand Up @@ -208,6 +213,8 @@ module.exports = function(grunt) {
'browserify:addonsMin',
'npm-react:release',
'npm-react:pack',
'npm-dom-property-config:release',
'npm-dom-property-config:pack',
'npm-react-tools:pack',
'copy:react_docs',
'compare_size'
Expand Down
77 changes: 77 additions & 0 deletions grunt/tasks/npm-dom-property-config.js
@@ -0,0 +1,77 @@
'use strict';

var fs = require('fs');
var grunt = require('grunt');

function npmGruntModule(opts) {
function buildRelease() {
// delete build/npm-dom-property-config for fresh start
grunt.file.exists(opts.dest) && grunt.file.delete(opts.dest);

// mkdir -p build/npm-dom-property-config/lib
grunt.file.mkdir(opts.lib);

// Copy npm-react/dom-property-config/**/* to build/npm-dom-property-config
var mappings = [].concat(
grunt.file.expandMapping('**/*', opts.dest, {cwd: opts.src})
);
mappings.forEach(function(mapping) {
var src = mapping.src[0];
var dest = mapping.dest;
if (grunt.file.isDir(src)) {
grunt.file.mkdir(dest);
} else {
grunt.file.copy(src, dest);
}
});

opts.files.forEach(function (fileName) {
grunt.file.copy(
opts.modSrc + fileName,
opts.lib + fileName
)
})

// modify build/react-core/package.json to set version ##
var pkg = grunt.file.readJSON(opts.dest + 'package.json');
pkg.version = grunt.config.data.pkg.version;
grunt.file.write(opts.dest + 'package.json', JSON.stringify(pkg, null, 2));
}

function packRelease() {
/*jshint validthis:true */
var done = this.async();
var spawnCmd = {
cmd: 'npm',
args: ['pack', opts.folderName],
opts: {
cwd: 'build/'
}
};
grunt.util.spawn(spawnCmd, function() {
var src = 'build/' + opts.name + '-' +
grunt.config.data.pkg.version + '.tgz';
var dest = 'build/' + opts.name + '.tgz';
fs.rename(src, dest, done);
});
}


return {
buildRelease: buildRelease,
packRelease: packRelease
};
}

module.exports = npmGruntModule({
folderName: 'npm-dom-property-config',
name: 'dom-property-config',
src: 'npm-react/dom-property-config/',
dest: 'build/npm-dom-property-config/',
modSrc: 'build/modules/',
lib: 'build/npm-dom-property-config/lib/',
files: [
'DefaultDOMPropertyConfig.js',
'DOMPropertyInjectionConstants.js'
]
});
145 changes: 145 additions & 0 deletions npm-react/dom-property-config/README.md
@@ -0,0 +1,145 @@
# dom-property-config
Copy link
Member

Choose a reason for hiding this comment

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

Let's move this out to the top level (npm-dom-property-config/) and not inside npm-react/ (try to keep each standalone module separate so we don't end up packaging them together)

Copy link
Author

Choose a reason for hiding this comment

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

good idea.


A set of dom property rendering instructions

`dom-property-config` contains the meta information on how to
correctly and performantly update any given property for a
DOM element.

## Example (property serializer)

You can use the `DOMAttributeNames` information to know how
to serialize DOM element properties to attributes correctly

```js
var DOMAttributeNames = require('dom-property-config').DOMAttributeNames

function elementSerializer(virtualElement) {
var attrs = []

Object.keys(virtualElement.properties).forEach(function (key) {
key = DOMAttributeNames[key] || key

attrs.push(key + '=' + '"' + virtualElement.properties[key] + '"')
})

return '<' + virtualElement.tagName +
attrs.length ? ' ' + attrs.join(' ') : '' +
'</' + virtualElement.tagName + '>'
}
```

## Example (property updater)

You can use `Properties` information to know how to correctly
mutate DOM properties on a live dom node

```js
var Constants = require('dom-property-config').Constants
var Properties = require('dom-property-config').Properties

function updateElement(element, virtualElement) {
Object.keys(virtualElement.properties).forEach(function (key) {
var strategy = Properties[key]

if (strategy & Constants.MUST_USE_ATTRIBUTE) {
element.setAttribute(key, virtualElement.properties[key])
} else if (strategy & Constants.MUST_USE_PROPERTY) {
element[key] = virtualElement.properties[key]
}
})
}
```

## Docs

### `var Properties = config.Properties`

The `Properties` object contains key value pairs that describe
the updating strategy that should be used to update the
named property on a DOM element.

The `key` is a virtual property name, not to be confused with
a DOM element property name. There is a mapping of virtual
property names to DOM element property names in
`config.DOMPropertyNames`

The `value` is either `null` which means it's not writable or
some binary OR combination of `MUST_USE_PROPERTY`,
`MUST_USE_ATTRIBUTE`, `HAS_BOOLEAN_VALUE`, `HAS_SIDE_EFFECTS`,
`HAS_NUMERIC_VALUE`, `HAS_POSITIVE_NUMERIC_VALUE`,
`HAS_OVERLOADED_BOOLEAN_VALUE`

You can use this object of key values to determine what kind
of DOM updating strategy you want to implement.

### `var Constants = config.Constants`

`Constants` contains the values of:

- `MUST_USE_ATTRIBUTE`
- `MUST_USE_PROPERTY`
- `HAS_BOOLEAN_VALUE`
- `HAS_SIDE_EFFECTS`
- `HAS_NUMERIC_VALUE`
- `HAS_POSITIVE_NUMERIC_VALUE`
- `HAS_OVERLOADED_BOOLEAN_VALUE`

`MUST_USE_ATTRIBUTE` means you should use `elem.setAttribute` to
update this property safely.

`MUST_USE_PROPERTY` means you should use `elem[key] = value` to
update this property safely.

`HAS_BOOLEAN_VALUE` means you know this property is a boolean.
this allows you to avoid serializing into `value=false`.

`HAS_SIDE_EFFECTS` means it is not safe to set this property.
you should special case this property in your algorithm

`HAS_NUMERIC_VALUE` means you know this property is a number.
this allows you to avoid serializing into `value=NaN`

`HAS_POSITIVE_NUMERIC_VALUE` means you know this property is
a positive number. this allows you to avoid serializing into
`value=-1`

`HAS_OVERLOADED_BOOLEAN_VALUE` means you know this property is
both a string and a boolean. this means you should not
serialize this property if it's `false`

### `var DOMAttributeNames = config.DOMAttributeNames`

The `DOMAttributeNames` object contains key value that describe
how to turn properties into attributes.

The `key` is a virtual property name and the `value` is the
relevant DOM element attribute name.

You can use this object to decided how to serialize your DOM
properties into a string of HTML attributes

## Installation

`npm install dom-property-config`

## Contributing

To work on this module please check out and clone the
[react](https://github.com/facebook/react) source code.

The implementation of this module can be found in

- [DefaultDomPropertyConfig](https://github.com/facebook/react/blob/master/src/browser/ui/dom/DefaultDOMPropertyConfig.js)
- [DOMPropertyInjectionConstants](https://github.com/facebook/react/blob/master/src/browser/ui/dom/DOMPropertyInjectionConstants.js)

The source code in this directly is auto generated and you
should branch and work on the files in the `react` src
directory directly.

## Contributors

- React maintainers
- Raynos

## Apache Licenced
1 change: 1 addition & 0 deletions npm-react/dom-property-config/dom-property-config.js
@@ -0,0 +1 @@
module.exports = require('./lib/DefaultDOMPropertyConfig.js');
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')

29 changes: 29 additions & 0 deletions npm-react/dom-property-config/package.json
@@ -0,0 +1,29 @@
{
"name": "dom-property-config",
"description": "A set of dom property rendering instructions",
"version": "",
"keywords": [
"react", "config", "attributes", "properties", "property", "attribute"
],
"homepage": "https://github.com/facebook/react/tree/master/npm-react/dom-property-config",
"bugs": "https://github.com/facebook/react/issues?labels=react-core",
"licenses": [
{
"type": "Apache-2.0",
"url": "http://www.apache.org/licenses/LICENSE-2.0"
}
],
"files": [
"README.md",
"dom-property-config.js",
"lib/"
],
"main": "dom-property-config.js",
"repository": {
"type": "git",
"url": "https://github.com/facebook/react"
},
"engines": {
"node": ">=0.10.0"
}
}
17 changes: 10 additions & 7 deletions src/browser/ui/dom/DOMProperty.js
Expand Up @@ -22,19 +22,22 @@
"use strict";

var invariant = require('invariant');
var DOMPropertyInjectionConstants = require('DOMPropertyInjectionConstants');

var DOMPropertyInjection = {
/**
* Mapping from normalized, camelcased property names to a configuration that
* specifies how the associated DOM property should be accessed or rendered.
*/
MUST_USE_ATTRIBUTE: 0x1,
MUST_USE_PROPERTY: 0x2,
HAS_SIDE_EFFECTS: 0x4,
HAS_BOOLEAN_VALUE: 0x8,
HAS_NUMERIC_VALUE: 0x10,
HAS_POSITIVE_NUMERIC_VALUE: 0x20 | 0x10,
HAS_OVERLOADED_BOOLEAN_VALUE: 0x40,
MUST_USE_ATTRIBUTE: DOMPropertyInjectionConstants.MUST_USE_ATTRIBUTE,
MUST_USE_PROPERTY: DOMPropertyInjectionConstants.MUST_USE_PROPERTY,
HAS_SIDE_EFFECTS: DOMPropertyInjectionConstants.HAS_SIDE_EFFECTS,
HAS_BOOLEAN_VALUE: DOMPropertyInjectionConstants.HAS_BOOLEAN_VALUE,
HAS_NUMERIC_VALUE: DOMPropertyInjectionConstants.HAS_NUMERIC_VALUE,
HAS_POSITIVE_NUMERIC_VALUE:
DOMPropertyInjectionConstants.HAS_POSITIVE_NUMERIC_VALUE,
HAS_OVERLOADED_BOOLEAN_VALUE:
DOMPropertyInjectionConstants.HAS_OVERLOADED_BOOLEAN_VALUE,

/**
* Inject some specialized knowledge about the DOM. This takes a config object
Expand Down
31 changes: 31 additions & 0 deletions src/browser/ui/dom/DOMPropertyInjectionConstants.js
@@ -0,0 +1,31 @@
/**
* Copyright 2013-2014 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
* @providesModule DOMPropertyInjectionConstants
*/

/*jslint bitwise: true*/

var DOMPropertyInjectionConstants = {
MUST_USE_ATTRIBUTE: 0x1,
MUST_USE_PROPERTY: 0x2,
HAS_SIDE_EFFECTS: 0x4,
HAS_BOOLEAN_VALUE: 0x8,
HAS_NUMERIC_VALUE: 0x10,
HAS_POSITIVE_NUMERIC_VALUE: 0x20 | 0x10,
HAS_OVERLOADED_BOOLEAN_VALUE: 0x40,
};

module.exports = DOMPropertyInjectionConstants;
19 changes: 10 additions & 9 deletions src/browser/ui/dom/DefaultDOMPropertyConfig.js
Expand Up @@ -20,17 +20,17 @@

"use strict";

var DOMProperty = require('DOMProperty');
var DOMPropertyInjectionConstants = require('DOMPropertyInjectionConstants');

var MUST_USE_ATTRIBUTE = DOMProperty.injection.MUST_USE_ATTRIBUTE;
var MUST_USE_PROPERTY = DOMProperty.injection.MUST_USE_PROPERTY;
var HAS_BOOLEAN_VALUE = DOMProperty.injection.HAS_BOOLEAN_VALUE;
var HAS_SIDE_EFFECTS = DOMProperty.injection.HAS_SIDE_EFFECTS;
var HAS_NUMERIC_VALUE = DOMProperty.injection.HAS_NUMERIC_VALUE;
var MUST_USE_ATTRIBUTE = DOMPropertyInjectionConstants.MUST_USE_ATTRIBUTE;
var MUST_USE_PROPERTY = DOMPropertyInjectionConstants.MUST_USE_PROPERTY;
var HAS_BOOLEAN_VALUE = DOMPropertyInjectionConstants.HAS_BOOLEAN_VALUE;
var HAS_SIDE_EFFECTS = DOMPropertyInjectionConstants.HAS_SIDE_EFFECTS;
var HAS_NUMERIC_VALUE = DOMPropertyInjectionConstants.HAS_NUMERIC_VALUE;
var HAS_POSITIVE_NUMERIC_VALUE =
DOMProperty.injection.HAS_POSITIVE_NUMERIC_VALUE;
DOMPropertyInjectionConstants.HAS_POSITIVE_NUMERIC_VALUE;
var HAS_OVERLOADED_BOOLEAN_VALUE =
DOMProperty.injection.HAS_OVERLOADED_BOOLEAN_VALUE;
DOMPropertyInjectionConstants.HAS_OVERLOADED_BOOLEAN_VALUE;

var DefaultDOMPropertyConfig = {
isCustomAttribute: RegExp.prototype.test.bind(
Expand Down Expand Up @@ -198,7 +198,8 @@ var DefaultDOMPropertyConfig = {
spellCheck: 'spellcheck',
srcDoc: 'srcdoc',
srcSet: 'srcset'
}
},
Constants: DOMPropertyInjectionConstants
Copy link
Member

Choose a reason for hiding this comment

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

I actually don't think we should export this here. We can make the npm package require the new module.

Copy link
Author

Choose a reason for hiding this comment

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

We need to export Constants somewhere.

An alternative to exporting it is require('dom-property-config/constants').

Constants contains the numbers you need to | and & against.

Copy link
Member

Choose a reason for hiding this comment

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

I was thinking main would require('./lib/DOMPropertyInjectionConstants') and re-export it however it wants. There's no real reason to export it here (especially html/svg split, which is incoming, btw)

Copy link
Author

Choose a reason for hiding this comment

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

Oh I see. good point.

};

module.exports = DefaultDOMPropertyConfig;