Skip to content

feat: support custom elements in React 16#78

Merged
remarkablemark merged 1 commit intoremarkablemark:masterfrom
bschlenk:support-custom-elements
Dec 12, 2018
Merged

feat: support custom elements in React 16#78
remarkablemark merged 1 commit intoremarkablemark:masterfrom
bschlenk:support-custom-elements

Conversation

@bschlenk
Copy link
Copy Markdown
Contributor

Closes #77

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.1%) to 99.291% when pulling 7b2c5a8 on bschlenk:support-custom-elements into bfe2023 on remarkablemark:master.

Copy link
Copy Markdown
Owner

@remarkablemark remarkablemark left a comment

Choose a reason for hiding this comment

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

Thanks for creating the PR @bschlenk, I added some comments. Let me know what you think

*/
function attributesToProps(attributes) {
attributes = attributes || {};
function attributesToProps(attributes = {}) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

revert: can we undo this line change since the library isn't transpiled and I wouldn't want the ES6 syntax to break for other users

/**
* Check if a given tag is a custom component.
*
* @see https://github.com/facebook/react/blob/master/packages/react-dom/src/shared/isCustomComponent.js
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

docs: could you use the following link instead: https://github.com/facebook/react/blob/v16.6.3/packages/react-dom/src/shared/isCustomComponent.js

This way, it's likely to 404 if it moves around

*/
function isCustomComponent(tagName, props) {
if (tagName.indexOf('-') === -1) {
return props && typeof props.is === 'string';
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

docs: do you mind adding a comment about the custom elements specification (https://www.w3.org/TR/2016/WD-custom-elements-20161013/#attr-is)

*
* @return {boolean}
*/
function reactSupportsUnknownAttributes() {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

perf: can we memoize the value here to minimize execution cycles?

var PRESERVE_CUSTOM_ATTRS = React.version.split('.')[0] >= 16;
// JS will coerce the value to number due to the comparison
// http://www.ecma-international.org/ecma-262/5.1/#sec-11.8.5

refactor: can we rename the value as PRESERVE_CUSTOM_ATTRS (so it follows the convention of a constant) and add a comment to the blog post above the line?

The only disadvantage of this approach is that the value isn't readonly or a constant so it's possible for others to override it

const attributesToProps = require('../lib/attributes-to-props');

describe('attributesToProps', () => {
let actualReactVersion;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

style: add newline

const { data, render } = require('./helpers/');

describe('dom-to-react parser', () => {
let actualReactVersion;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

style: add newline

});

describe('utilities.reactSupportsUnknownAttributes', () => {
let actualVersion;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

style: add newline

@remarkablemark remarkablemark added the feature New feature or request label Dec 4, 2018
@remarkablemark remarkablemark dismissed their stale review December 12, 2018 03:53

To not block this PR from getting merged, I'll make the fixes/changes on my side

@remarkablemark remarkablemark merged commit 9eeef55 into remarkablemark:master Dec 12, 2018
d-lazarev pushed a commit to d-lazarev/html-react-parser that referenced this pull request Apr 5, 2019
…ments

feat: support custom elements in React 16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants