Skip to content

[Fiber] Support SVG #8490

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.

Sign up for GitHub

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

Merged
gaearon merged 31 commits into from
Dec 8, 2016
Merged

[Fiber] Support SVG #8490

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
4f7455d
Test that SVG elements get created with the right namespace
gaearon Nov 24, 2016
ebb80e8
Pass root to the renderer methods
gaearon Nov 24, 2016
ef639f6
Keep track of host instances and containers
gaearon Nov 24, 2016
9aaf31b
Keep instances instead of fibers on the stack
gaearon Nov 25, 2016
d94aee1
Create text instances in begin phase
gaearon Nov 25, 2016
db2018c
Create instance before bailing on offscreen children
gaearon Nov 25, 2016
2a090c1
Tweak magic numbers in incremental tests
gaearon Nov 25, 2016
6899b38
Only push newly created nodes on the parent stack
gaearon Nov 25, 2016
b08ce9d
Fix lint
gaearon Nov 25, 2016
44189e0
Fix Flow
gaearon Nov 25, 2016
77fb926
Use the same destructuring style in scheduler as everywhere else
gaearon Nov 25, 2016
ce39123
Remove branches that don't seem to run anymore
gaearon Nov 25, 2016
dfcf07b
Be explicit about the difference between type and tag
gaearon Nov 25, 2016
93de77f
Save and restore host context when pushing and popping portals
gaearon Nov 25, 2016
6ab662c
Revert parent context and adding children in the begin phase
gaearon Nov 26, 2016
840285b
Add a test for SVG updates
gaearon Nov 28, 2016
c2b8888
Record tests
gaearon Nov 29, 2016
8dbd419
Read ownerDocument from the root container instance
gaearon Nov 29, 2016
c332d6f
Track namespaces instead of creating instances early
gaearon Nov 29, 2016
836f829
Pop child context before reading own context and clarify API
gaearon Nov 29, 2016
a37d5e9
Give SVG namespace to <svg> itself
gaearon Nov 29, 2016
1b3178b
Don't allocate unnecessarily when reconciling portals
gaearon Nov 29, 2016
84e23ca
Add more tests for edge cases
gaearon Dec 2, 2016
aa57380
Fix up math namespace
gaearon Dec 2, 2016
b31cb3d
Maintain a separate container stack
gaearon Dec 3, 2016
dfd37c8
Fix rebase mistakes
gaearon Dec 3, 2016
4240833
Unwind context on errors
gaearon Dec 3, 2016
f80f094
Merge master into fiber-svg-3
gaearon Dec 8, 2016
4ba9d64
Reset the container state when reusing the object
gaearon Dec 8, 2016
f64f786
Add getChildHostContext() to ReactART
gaearon Dec 8, 2016
79074b1
Record tests
gaearon Dec 8, 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
8 changes: 8 additions & 0 deletions scripts/fiber/tests-passing.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,12 @@ src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
* should render one portal
* should render many portals
* should render nested portals
* should keep track of namespace across portals (simple)
* should keep track of namespace across portals (medium)
* should keep track of namespace across portals (complex)
* should unwind namespaces on uncaught errors
* should unwind namespaces on caught errors
* should unwind namespaces on caught errors in a portal
* 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
Expand Down Expand Up @@ -669,6 +675,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
Expand Down
4 changes: 4 additions & 0 deletions src/renderers/art/ReactARTFiber.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,10 @@ const ARTRenderer = ReactFiberReconciler({
// Noop
},

getChildHostContext() {
return null;
},

Copy link
Collaborator

@sebmarkbage sebmarkbage Dec 8, 2016

Choose a reason for hiding this comment

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

We'll need to replicate this to the separate repo. cc @bvaughn

Copy link
Contributor

@bvaughn bvaughn Dec 8, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Roger that. I'll append it to my existing sync PR.

Edit: Added to reactjs/react-art/pull/109 and merged.

Copy link
Contributor

@bvaughn bvaughn Dec 8, 2016

Choose a reason for hiding this comment

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

nit: I think it would be nice for us to alpha-sort HostConfig props to make it easier to scan for differences between fiber renderers. (At least it helps when things are changing fairly often due to fiber being in-flux.)

Copy link
Collaborator

@sebmarkbage sebmarkbage Dec 8, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

We should make these optional so that they don't have to be added to every renderer. That way we can also do things like:

const { getChildHostContext } = config;
if (getChildHostContext) {
  var data = getSomeData();
  return getChildHostContext(data);
}

That way if the compiler knows that it is always undefined, it can dead-code eliminate everything in the if statement. That's part of the reason I'd like to compile the renderer and host config together into one unit instead of making them share code.

scheduleAnimationCallback: window.requestAnimationFrame,

scheduleDeferredCallback: window.requestIdleCallback,
Expand Down
43 changes: 31 additions & 12 deletions src/renderers/dom/fiber/ReactDOMFiber.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ var warning = require('warning');

var {
createElement,
getChildNamespace,
setInitialProperties,
updateProperties,
} = ReactDOMFiberComponent;
Expand Down Expand Up @@ -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);
Expand All @@ -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;
},
Expand All @@ -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(
Expand All @@ -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();
// 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);
},

shouldSetTextContent(props : Props) : boolean {
Expand Down
69 changes: 39 additions & 30 deletions src/renderers/dom/fiber/ReactDOMFiberComponent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ var CHILDREN = 'children';
var STYLE = 'style';
var HTML = '__html';

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

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

Expand Down Expand Up @@ -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;
}
// 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
Expand All @@ -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
);
}

Expand Down
Loading