Skip to content

[Fiber] Support SVG by adding a container stack #8417

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 22 commits into from
Closed
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
18a9869
Test that SVG elements get created with the right namespace
gaearon Nov 24, 2016
b8b2638
Pass root to the renderer methods
gaearon Nov 24, 2016
a450a08
Keep track of host instances and containers
gaearon Nov 24, 2016
3bc1b07
Keep instances instead of fibers on the stack
gaearon Nov 25, 2016
21e7b04
Create text instances in begin phase
gaearon Nov 25, 2016
a810979
Create instance before bailing on offscreen children
gaearon Nov 25, 2016
62b6241
Tweak magic numbers in incremental tests
gaearon Nov 25, 2016
b8846ad
Only push newly created nodes on the parent stack
gaearon Nov 25, 2016
2c5caa0
Fix lint
gaearon Nov 25, 2016
ae91e59
Fix Flow
gaearon Nov 25, 2016
063199e
Use the same destructuring style in scheduler as everywhere else
gaearon Nov 25, 2016
00c75b2
Remove branches that don't seem to run anymore
gaearon Nov 25, 2016
7debd3a
Be explicit about the difference between type and tag
gaearon Nov 25, 2016
e6dd706
Save and restore host context when pushing and popping portals
gaearon Nov 25, 2016
0d41720
Revert parent context and adding children in the begin phase
gaearon Nov 26, 2016
dd93f0c
Add a test for SVG updates
gaearon Nov 28, 2016
94cd7db
Record tests
gaearon Nov 29, 2016
2099a2a
Read ownerDocument from the root container instance
gaearon Nov 29, 2016
c08a24a
Track namespaces instead of creating instances early
gaearon Nov 29, 2016
1b25dfb
Pop child context before reading own context and clarify API
gaearon Nov 29, 2016
5ee8dcf
Give SVG namespace to <svg> itself
gaearon Nov 29, 2016
5f4f36d
Don't allocate unnecessarily when reconciling portals
gaearon Nov 29, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions scripts/fiber/tests-passing.txt
Original file line number Diff line number Diff line change
@@ -504,6 +504,7 @@ src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
* should render one portal
* should render many portals
* should render nested portals
* should not apply SVG mode across portals
* should pass portal context when rendering subtree elsewhere
* should update portal context if it changes due to setState
* should update portal context if it changes due to re-render
@@ -654,6 +655,8 @@ src/renderers/dom/shared/__tests__/ReactDOMInvalidARIAHook-test.js

src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js
* creates initial namespaced markup
* creates elements with SVG namespace inside SVG tag during mount
* creates elements with SVG namespace inside SVG tag during update

src/renderers/dom/shared/__tests__/ReactDOMTextComponent-test.js
* updates a mounted text component in place
43 changes: 31 additions & 12 deletions src/renderers/dom/fiber/ReactDOMFiber.js
Original file line number Diff line number Diff line change
@@ -33,6 +33,7 @@ var warning = require('warning');

var {
createElement,
getChildNamespace,
setInitialProperties,
updateProperties,
} = ReactDOMFiberComponent;
@@ -60,6 +61,11 @@ let selectionInformation : ?mixed = null;

var DOMRenderer = ReactFiberReconciler({

getChildHostContext(parentHostContext : string | null, type : string) {
const parentNamespace = parentHostContext;
return getChildNamespace(parentNamespace, type);
},

prepareForCommit() : void {
eventsEnabled = ReactBrowserEventEmitter.isEnabled();
ReactBrowserEventEmitter.setEnabled(false);
@@ -76,11 +82,11 @@ var DOMRenderer = ReactFiberReconciler({
createInstance(
type : string,
props : Props,
internalInstanceHandle : Object
rootContainerInstance : Container,
hostContext : string | null,
internalInstanceHandle : Object,
) : Instance {
const root = document.documentElement; // HACK

const domElement : Instance = createElement(type, props, root);
const domElement : Instance = createElement(type, props, rootContainerInstance, hostContext);
precacheFiberNode(internalInstanceHandle, domElement);
return domElement;
},
@@ -89,10 +95,18 @@ var DOMRenderer = ReactFiberReconciler({
parentInstance.appendChild(child);
},

finalizeInitialChildren(domElement : Instance, type : string, props : Props) : void {
const root = document.documentElement; // HACK

setInitialProperties(domElement, type, props, root);
finalizeInitialChildren(
domElement : Instance,
props : Props,
rootContainerInstance : Container,
) : void {
// TODO: we normalize here because DOM renderer expects tag to be lowercase.
// We can change DOM renderer to compare special case against upper case,
// and use tagName (which is upper case for HTML DOM elements). Or we could
// let the renderer "normalize" the fiber type so we don't have to read
// the type from DOM. However we need to remember SVG is case-sensitive.
var tag = domElement.tagName.toLowerCase();
setInitialProperties(domElement, tag, props, rootContainerInstance);
},

prepareUpdate(
@@ -107,14 +121,19 @@ var DOMRenderer = ReactFiberReconciler({
domElement : Instance,
oldProps : Props,
newProps : Props,
internalInstanceHandle : Object
rootContainerInstance : Container,
internalInstanceHandle : Object,
) : void {
var type = domElement.tagName.toLowerCase(); // HACK
var root = document.documentElement; // HACK
// TODO: we normalize here because DOM renderer expects tag to be lowercase.
// We can change DOM renderer to compare special case against upper case,
// and use tagName (which is upper case for HTML DOM elements). Or we could
// let the renderer "normalize" the fiber type so we don't have to read
// the type from DOM. However we need to remember SVG is case-sensitive.
var tag = domElement.tagName.toLowerCase();
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is likely going to be an issue to do for every update. We might want to consider just banning non-lowercase uses instead.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can omit this change if it's better to discuss separately.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also, we already doing this in commitUpdate() before my PR. So I don't see why doing this during mounting is worse. (We should fix both?)

// Update the internal instance handle so that we know which props are
// the current ones.
precacheFiberNode(internalInstanceHandle, domElement);
updateProperties(domElement, type, oldProps, newProps, root);
updateProperties(domElement, tag, oldProps, newProps, rootContainerInstance);
},

createTextInstance(text : string, internalInstanceHandle : Object) : TextInstance {
69 changes: 39 additions & 30 deletions src/renderers/dom/fiber/ReactDOMFiberComponent.js
Original file line number Diff line number Diff line change
@@ -46,6 +46,11 @@ var CHILDREN = 'children';
var STYLE = 'style';
var HTML = '__html';

var {
svg: SVG_NAMESPACE,
math: MATH_NAMESPACE,
} = DOMNamespaces;

// Node type for document fragments (Node.DOCUMENT_FRAGMENT_NODE).
var DOC_FRAGMENT_TYPE = 11;

@@ -451,45 +456,49 @@ function updateDOMProperties(
}
}

var ReactDOMFiberComponent = {
// Assumes there is no parent namespace.
function getIntrinsicNamespace(type : string) : string | null {
switch (type) {
case 'svg':
return SVG_NAMESPACE;
case 'math':
return MATH_NAMESPACE;
default:
return null;
}
}

// TODO: Use this to keep track of changes to the host context and use this
// to determine whether we switch to svg and back.
// TODO: Does this need to check the current namespace? In case these tags
// happen to be valid in some other namespace.
isNewHostContainer(tag : string) {
return tag === 'svg' || tag === 'foreignobject';
var ReactDOMFiberComponent = {
getChildNamespace(parentNamespace : string | null, type : string) : string | null {
if (parentNamespace == null) {
// No parent namespace: potential entry point.
return getIntrinsicNamespace(type);
}
if (parentNamespace === SVG_NAMESPACE && type === 'foreignObject') {
// We're leaving SVG.
return null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe the foreignObject itself should be the SVG namespace but then get next namespace underneath it should be HTML.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add an assertion that the <svg /> tag itself has the SVG namespace?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Shame on me!

}
// By default, pass namespace below.
return parentNamespace;
},

createElement(
tag : string,
type : string,
props : Object,
rootContainerElement : Element
rootContainerElement : Element,
parentNamespace : string | null
) : Element {
validateDangerousTag(tag);
validateDangerousTag(type);
// TODO:
// tag.toLowerCase(); Do we need to apply lower case only on non-custom elements?
// const tag = type.toLowerCase(); Do we need to apply lower case only on non-custom elements?

// We create tags in the namespace of their parent container, except HTML
// tags get no namespace.
var namespaceURI = rootContainerElement.namespaceURI;
if (namespaceURI == null ||
namespaceURI === DOMNamespaces.svg &&
rootContainerElement.tagName === 'foreignObject') {
namespaceURI = DOMNamespaces.html;
}
if (namespaceURI === DOMNamespaces.html) {
if (tag === 'svg') {
namespaceURI = DOMNamespaces.svg;
} else if (tag === 'math') {
namespaceURI = DOMNamespaces.mathml;
}
// TODO: Make this a new root container element.
}

var ownerDocument = rootContainerElement.ownerDocument;
var domElement : Element;
if (namespaceURI === DOMNamespaces.html) {
var namespaceURI = parentNamespace || getIntrinsicNamespace(type);
if (namespaceURI == null) {
const tag = type.toLowerCase();
if (tag === 'script') {
// Create the script via .innerHTML so its "parser-inserted" flag is
// set to true and it does not execute
@@ -499,17 +508,17 @@ var ReactDOMFiberComponent = {
var firstChild = ((div.firstChild : any) : HTMLScriptElement);
domElement = div.removeChild(firstChild);
} else if (props.is) {
domElement = ownerDocument.createElement(tag, props.is);
domElement = ownerDocument.createElement(type, props.is);
} else {
// Separate else branch instead of using `props.is || undefined` above becuase of a Firefox bug.
// See discussion in https://github.com/facebook/react/pull/6896
// and discussion in https://bugzilla.mozilla.org/show_bug.cgi?id=1276240
domElement = ownerDocument.createElement(tag);
domElement = ownerDocument.createElement(type);
}
} else {
domElement = ownerDocument.createElementNS(
namespaceURI,
tag
type
);
}

56 changes: 56 additions & 0 deletions src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
Original file line number Diff line number Diff line change
@@ -333,6 +333,62 @@ describe('ReactDOMFiber', () => {
expect(container.innerHTML).toBe('');
});

it('should not apply SVG mode across portals', () => {
var portalContainer = document.createElement('div');

ReactDOM.render(
<svg>
<image xlinkHref="http://i.imgur.com/w7GCRPb.png" />
{ReactDOM.unstable_createPortal(
<div>portal</div>,
portalContainer
)}
<image xlinkHref="http://i.imgur.com/w7GCRPb.png" />
</svg>,
container
);

const div = portalContainer.childNodes[0];
const image1 = container.firstChild.childNodes[0];
const image2 = container.firstChild.childNodes[1];
expect(div.namespaceURI).toBe('http://www.w3.org/1999/xhtml');
expect(div.tagName).toBe('DIV');
expect(image1.namespaceURI).toBe('http://www.w3.org/2000/svg');
expect(image1.tagName).toBe('image');
expect(
image1.getAttributeNS('http://www.w3.org/1999/xlink', 'href')
).toBe('http://i.imgur.com/w7GCRPb.png');
expect(image2.namespaceURI).toBe('http://www.w3.org/2000/svg');
expect(image2.tagName).toBe('image');
expect(
image2.getAttributeNS('http://www.w3.org/1999/xlink', 'href')
).toBe('http://i.imgur.com/w7GCRPb.png');

ReactDOM.render(
<svg>
<image xlinkHref="http://i.imgur.com/w7GCRPb.png" />
{ReactDOM.unstable_createPortal(
<span>portal</span>,
portalContainer
)}
<g />
</svg>,
container
);

const span = portalContainer.childNodes[0];
expect(span.namespaceURI).toBe('http://www.w3.org/1999/xhtml');
expect(span.tagName).toBe('SPAN');
expect(container.firstChild.childNodes[0]).toBe(image1);
const g = container.firstChild.childNodes[1];
expect(g.namespaceURI).toBe('http://www.w3.org/2000/svg');
expect(g.tagName).toBe('g');

ReactDOM.unmountComponentAtNode(container);
expect(portalContainer.innerHTML).toBe('');
expect(container.innerHTML).toBe('');
});

it('should pass portal context when rendering subtree elsewhere', () => {
var portalContainer = document.createElement('div');

98 changes: 98 additions & 0 deletions src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js
Original file line number Diff line number Diff line change
@@ -12,12 +12,14 @@
'use strict';

var React;
var ReactDOM;
var ReactDOMServer;

describe('ReactDOMSVG', () => {

beforeEach(() => {
React = require('React');
ReactDOM = require('ReactDOM');
ReactDOMServer = require('ReactDOMServer');
});

@@ -30,4 +32,100 @@ describe('ReactDOMSVG', () => {
expect(markup).toContain('xlink:href="http://i.imgur.com/w7GCRPb.png"');
});

it('creates elements with SVG namespace inside SVG tag during mount', () => {
var node = document.createElement('div');
var div, foreignDiv, foreignObject, g, image, image2, p, svg;
ReactDOM.render(
<div>
<svg ref={el => svg = el}>
<g ref={el => g = el} strokeWidth="5">
<image ref={el => image = el} xlinkHref="http://i.imgur.com/w7GCRPb.png" />
<foreignObject ref={el => foreignObject = el}>
<div ref={el => foreignDiv = el} />
</foreignObject>
</g>
</svg>
<p ref={el => p = el}>
<svg>
<image ref={el => image2 = el} xlinkHref="http://i.imgur.com/w7GCRPb.png" />
</svg>
</p>
<div ref={el => div = el} />
</div>,
node
);
// SVG tagName is case sensitive.
expect(svg.namespaceURI).toBe('http://www.w3.org/2000/svg');
expect(svg.tagName).toBe('svg');
expect(g.namespaceURI).toBe('http://www.w3.org/2000/svg');
expect(g.tagName).toBe('g');
expect(g.getAttribute('stroke-width')).toBe('5');
expect(image.namespaceURI).toBe('http://www.w3.org/2000/svg');
expect(image.tagName).toBe('image');
expect(
image.getAttributeNS('http://www.w3.org/1999/xlink', 'href')
).toBe('http://i.imgur.com/w7GCRPb.png');
expect(foreignObject.namespaceURI).toBe('http://www.w3.org/2000/svg');
expect(foreignObject.tagName).toBe('foreignObject');
expect(image2.namespaceURI).toBe('http://www.w3.org/2000/svg');
expect(image2.tagName).toBe('image');
expect(
image2.getAttributeNS('http://www.w3.org/1999/xlink', 'href')
).toBe('http://i.imgur.com/w7GCRPb.png');
// DOM tagName is capitalized by browsers.
expect(p.namespaceURI).toBe('http://www.w3.org/1999/xhtml');
expect(p.tagName).toBe('P');
expect(div.namespaceURI).toBe('http://www.w3.org/1999/xhtml');
expect(div.tagName).toBe('DIV');
expect(foreignDiv.namespaceURI).toBe('http://www.w3.org/1999/xhtml');
expect(foreignDiv.tagName).toBe('DIV');
});

it('creates elements with SVG namespace inside SVG tag during update', () => {
var inst, foreignObject, foreignDiv, g, image, svg;

class App extends React.Component {
state = {step: 0};
render() {
inst = this;
const {step} = this.state;
if (step === 0) {
return null;
}
return (
<g ref={el => g = el} strokeWidth="5">
<image ref={el => image = el} xlinkHref="http://i.imgur.com/w7GCRPb.png" />
<foreignObject ref={el => foreignObject = el}>
<div ref={el => foreignDiv = el} />
</foreignObject>
</g>
);
}
}

var node = document.createElement('div');
ReactDOM.render(
<svg ref={el => svg = el}>
<App />
</svg>,
node
);
inst.setState({step: 1});

expect(svg.namespaceURI).toBe('http://www.w3.org/2000/svg');
expect(svg.tagName).toBe('svg');
expect(g.namespaceURI).toBe('http://www.w3.org/2000/svg');
expect(g.tagName).toBe('g');
expect(g.getAttribute('stroke-width')).toBe('5');
expect(image.namespaceURI).toBe('http://www.w3.org/2000/svg');
expect(image.tagName).toBe('image');
expect(
image.getAttributeNS('http://www.w3.org/1999/xlink', 'href')
).toBe('http://i.imgur.com/w7GCRPb.png');
expect(foreignObject.namespaceURI).toBe('http://www.w3.org/2000/svg');
expect(foreignObject.tagName).toBe('foreignObject');
expect(foreignDiv.namespaceURI).toBe('http://www.w3.org/1999/xhtml');
expect(foreignDiv.tagName).toBe('DIV');
});

});
Loading