on master help sign in
Make `Node` opaque

I don't think it needs to have its internal representation exposed to users, especially since I intend to make breaking changes to that representation. Would rather start exposing its internals as user requests are made.

commit3b9910f3bf313bb1b2465025ef8a98ea8e4810e1 authorNick Fitzgerald <fitzgen@gmail.com> dateMon 25 Mar 2019, 15:39 (8 years ago) parent4e428e0
8 files changed+145−122
.travis.yml+2−2 Cargo.toml+9−4 src/change_list.rs+24−18 src/events.rs+2−2 src/lib.rs+29−2 src/node.rs+49−73 src/vdom.rs+14−11 tests/web/main.rs+16−10
.travis.yml+2−2
@@ -32,12 +32,12 @@ matrix:
3232 rust: stable
3333 addons:
3434 firefox: latest
35− script: wasm-pack test --firefox --headless
35+ script: wasm-pack test --firefox --headless -- --features xxx-unstable-internal-use-only
3636 - name: "headless browser tests (chrome)"
3737 rust: stable
3838 addons:
3939 chrome: stable
40− script: wasm-pack test --chrome --headless
40+ script: wasm-pack test --chrome --headless -- --features xxx-unstable-internal-use-only
4141 - name: "native tests"
4242 rust: stable
4343 install: echo "no install"
Cargo.toml+9−4
@@ -13,6 +13,15 @@ version = "0.1.0"
1313[badges]
1414travis-ci = { repository = "fitzgen/dodrio" }
1515
16+[lib]
17+path = "src/lib.rs"
18+bench = false
19+
20+[[test]]
21+name = "web"
22+path = "tests/web/main.rs"
23+required-features = ["xxx-unstable-internal-use-only"]
24+
1625[[bench]]
1726name = "benches"
1827path = "benches/benches.rs"
@@ -64,10 +73,6 @@ features = [
6473[target.'cfg(not(target_arch = "wasm32"))'.dev-dependencies]
6574criterion = "0.2.10"
6675
67−[lib]
68−path = "src/lib.rs"
69−bench = false
70−
7176[profile.release]
7277incremental = false
7378lto = true
src/change_list.rs+24−18
@@ -5,7 +5,7 @@ use std::fmt;
55
66pub mod js {
77 cfg_if::cfg_if! {
8− if #[cfg(feature = "xxx-unstable-internal-use-only")] {
8+ if #[cfg(all(feature = "xxx-unstable-internal-use-only", not(target_arch = "wasm32")))] {
99 #[derive(Clone, Debug)]
1010 pub struct ChangeList {}
1111 impl ChangeList {
@@ -107,23 +107,6 @@ impl ChangeList {
107107 }
108108 }
109109
110− #[cfg(feature = "xxx-unstable-internal-use-only")]
111− pub(crate) fn apply_changes(&mut self) {
112− // Do nothing...
113− }
114−
115− #[cfg(not(feature = "xxx-unstable-internal-use-only"))]
116− pub(crate) fn apply_changes(&mut self) {
117− let js = &self.js;
118− unsafe {
119− self.bump.each_allocated_chunk(|ch| {
120− js.add_change_list_range(ch.as_ptr() as usize, ch.len());
121− });
122− }
123− js.apply_changes(wasm_bindgen::memory());
124− self.bump.reset();
125− }
126−
127110 pub(crate) fn init_events_trampoline(&mut self, trampoline: crate::EventsTrampoline) {
128111 debug_assert!(self.events_trampoline.is_none());
129112 self.js.init_events_trampoline(&trampoline);
@@ -131,6 +114,29 @@ impl ChangeList {
131114 }
132115}
133116
117+cfg_if::cfg_if! {
118+ if #[cfg(all(feature = "xxx-unstable-internal-use-only", not(target_arch = "wasm32")))] {
119+ impl ChangeList {
120+ pub(crate) fn apply_changes(&mut self) {
121+ // Do nothing...
122+ }
123+ }
124+ } else {
125+ impl ChangeList {
126+ pub(crate) fn apply_changes(&mut self) {
127+ let js = &self.js;
128+ unsafe {
129+ self.bump.each_allocated_chunk(|ch| {
130+ js.add_change_list_range(ch.as_ptr() as usize, ch.len());
131+ });
132+ }
133+ js.apply_changes(wasm_bindgen::memory());
134+ self.bump.reset();
135+ }
136+ }
137+ }
138+}
139+
134140#[repr(u32)]
135141#[derive(Clone, Copy, Debug)]
136142enum ChangeDiscriminant {
src/events.rs+2−2
@@ -4,7 +4,7 @@ use std::cell::RefCell;
44use std::rc::{Rc, Weak};
55
66cfg_if::cfg_if! {
7− if #[cfg(feature = "xxx-unstable-internal-use-only")] {
7+ if #[cfg(all(feature = "xxx-unstable-internal-use-only", not(target_arch = "wasm32")))] {
88 #[derive(Debug)]
99 pub(crate) struct EventsRegistry {}
1010 impl EventsRegistry {
@@ -18,7 +18,7 @@ cfg_if::cfg_if! {
1818 pub(crate) fn clear_active_listeners(&mut self) {}
1919 }
2020 } else {
21− use crate::{ListenerCallback, vdom::VdomWeak};
21+ use crate::{node::ListenerCallback, vdom::VdomWeak};
2222 use std::collections::HashMap;
2323 use std::fmt;
2424 use std::mem;
src/lib.rs+29−2
@@ -61,6 +61,26 @@ cfg_if::cfg_if! {
6161 }
6262}
6363
64+// A macro to expose items unstably, only for internal/testing usage.
65+cfg_if::cfg_if! {
66+ if #[cfg(feature = "xxx-unstable-internal-use-only")] {
67+ macro_rules! pub_unstable_internal {
68+ ( $(#[$attr:meta])* pub(crate) $( $thing:tt )* ) => {
69+ #[doc(hidden)]
70+ $( #[$attr] )*
71+ pub $( $thing )*
72+ }
73+ }
74+ } else {
75+ macro_rules! pub_unstable_internal {
76+ ( $(#[$attr:meta])* pub(crate) $( $thing:tt )* ) => {
77+ $( #[$attr] )*
78+ pub(crate) $( $thing )*
79+ }
80+ }
81+ }
82+}
83+
6484// Only `pub` so that the wasm-bindgen bindings work.
6585#[doc(hidden)]
6686pub mod change_list;
@@ -74,13 +94,14 @@ mod vdom;
7494pub mod builder;
7595
7696// Re-export items at the top level.
77−pub use self::node::{Attribute, ElementNode, Listener, ListenerCallback, Node, TextNode};
97+pub use self::node::{Attribute, Listener, Node};
7898pub use self::render::{Render, RootRender};
7999pub use self::render_context::RenderContext;
80100pub use self::vdom::{Vdom, VdomWeak};
81101
102+// Polyfill some Web stuff for benchmarking...
82103cfg_if::cfg_if! {
83− if #[cfg(feature = "xxx-unstable-internal-use-only")] {
104+ if #[cfg(all(feature = "xxx-unstable-internal-use-only", not(target_arch = "wasm32")))] {
84105 /// An element node in the physical DOM.
85106 pub type Element = ();
86107
@@ -92,3 +113,9 @@ cfg_if::cfg_if! {
92113 pub(crate) type EventsTrampoline = wasm_bindgen::closure::Closure<Fn(web_sys::Event, u32, u32)>;
93114 }
94115}
116+
117+cfg_if::cfg_if! {
118+ if #[cfg(feature = "xxx-unstable-internal-use-only")] {
119+ pub use self::node::{ElementNode, NodeKind, TextNode};
120+ }
121+}
src/node.rs+49−73
@@ -4,32 +4,49 @@ use std::fmt;
44use std::iter;
55use std::mem;
66
7−/// A node is either a text node or an element.
7+/// A virtual DOM node.
88#[derive(Debug, Clone)]
9−pub enum Node<'a> {
10− /// A text node.
11− Text(TextNode<'a>),
9+pub struct Node<'a> {
10+ #[cfg(feature = "xxx-unstable-internal-use-only")]
11+ #[doc(hidden)]
12+ pub kind: NodeKind<'a>,
1213
13− /// An element potentially with attributes and children.
14− Element(ElementNode<'a>),
14+ #[cfg(not(feature = "xxx-unstable-internal-use-only"))]
15+ pub(crate) kind: NodeKind<'a>,
1516}
1617
17−/// Text nodes are just a string of text. They cannot have attributes or
18−/// children.
19−#[derive(Debug, Clone)]
20−pub struct TextNode<'a> {
21− pub(crate) text: &'a str,
18+pub_unstable_internal! {
19+ /// A node is either a text node or an element.
20+ #[derive(Debug, Clone)]
21+ pub(crate) enum NodeKind<'a> {
22+ /// A text node.
23+ Text(TextNode<'a>),
24+
25+ /// An element potentially with attributes and children.
26+ Element(ElementNode<'a>),
27+ }
2228}
2329
24−/// Elements have a tag name, zero or more attributes, and zero or more
25−/// children.
26−#[derive(Debug, Clone)]
27−pub struct ElementNode<'a> {
28− pub(crate) tag_name: &'a str,
29− pub(crate) listeners: &'a [Listener<'a>],
30− pub(crate) attributes: &'a [Attribute<'a>],
31− pub(crate) children: &'a [Node<'a>],
32− pub(crate) namespace: Option<&'a str>,
30+pub_unstable_internal! {
31+ /// Text nodes are just a string of text. They cannot have attributes or
32+ /// children.
33+ #[derive(Debug, Clone)]
34+ pub(crate) struct TextNode<'a> {
35+ pub text: &'a str,
36+ }
37+}
38+
39+pub_unstable_internal! {
40+ /// Elements have a tag name, zero or more attributes, and zero or more
41+ /// children.
42+ #[derive(Debug, Clone)]
43+ pub(crate) struct ElementNode<'a> {
44+ pub tag_name: &'a str,
45+ pub listeners: &'a [Listener<'a>],
46+ pub attributes: &'a [Attribute<'a>],
47+ pub children: &'a [Node<'a>],
48+ pub namespace: Option<&'a str>,
49+ }
3350}
3451
3552/// An event listener callback function.
@@ -39,7 +56,7 @@ pub struct ElementNode<'a> {
3956/// 1. The virtual DOM's root rendering component.
4057/// 2. A capability to scheduler virtual DOM re-rendering.
4158/// 3. The event that occurred.
42−pub type ListenerCallback<'a> =
59+pub(crate) type ListenerCallback<'a> =
4360 &'a (dyn Fn(&mut dyn RootRender, VdomWeak, web_sys::Event) + 'static);
4461
4562/// An event listener.
@@ -121,35 +138,23 @@ impl<'a> Node<'a> {
121138 let attributes: &'a Attributes = bump.alloc(attributes);
122139 let attributes: &'a [Attribute<'a>] = attributes.as_ref();
123140
124− Node::Element(ElementNode {
125− tag_name,
126− listeners,
127− attributes,
128− children,
129− namespace,
130− })
131− }
132−
133− /// Is this node a text node?
134− pub fn is_text(&self) -> bool {
135− match self {
136− Node::Text(_) => true,
137− _ => false,
138− }
139− }
140−
141− /// Is this node an element?
142− pub fn is_element(&self) -> bool {
143− match self {
144− Node::Element { .. } => true,
145− _ => false,
141+ Node {
142+ kind: NodeKind::Element(ElementNode {
143+ tag_name,
144+ listeners,
145+ attributes,
146+ children,
147+ namespace,
148+ }),
146149 }
147150 }
148151
149152 /// Construct a new text node with the given text.
150153 #[inline]
151154 pub(crate) fn text(text: &'a str) -> Node<'a> {
152− Node::Text(TextNode { text })
155+ Node {
156+ kind: NodeKind::Text(TextNode { text }),
157+ }
153158 }
154159}
155160
@@ -169,35 +174,6 @@ impl<'a> IntoIterator for Node<'a> {
169174 }
170175}
171176
172−impl<'a> TextNode<'a> {
173− /// Get this text node's text content.
174− pub fn text(&self) -> &'a str {
175− self.text
176− }
177−}
178−
179−impl<'a> ElementNode<'a> {
180− /// Get this element's tag name.
181− pub fn tag_name(&self) -> &'a str {
182− self.tag_name
183− }
184−
185− /// Get this element's attributes.
186− pub fn attributes(&self) -> &'a [Attribute<'a>] {
187− self.attributes
188− }
189−
190− /// Get this element's attributes.
191− pub fn children(&self) -> &'a [Node<'a>] {
192− self.children
193− }
194−
195− /// Get this element's namespace.
196− pub fn namespace(&self) -> Option<&'a str> {
197− self.namespace
198− }
199−}
200−
201177union CallbackFatPtr<'a> {
202178 callback: ListenerCallback<'a>,
203179 parts: (u32, u32),
src/vdom.rs+14−11
@@ -1,5 +1,5 @@
11use super::change_list::ChangeList;
2−use super::node::{Attribute, ElementNode, Listener, Node, TextNode};
2+use super::node::{Attribute, ElementNode, Listener, Node, NodeKind, TextNode};
33use super::RootRender;
44use crate::events::EventsRegistry;
55use crate::RenderContext;
@@ -115,7 +115,7 @@ impl Drop for VdomInnerExclusive {
115115}
116116
117117cfg_if::cfg_if! {
118− if #[cfg(feature = "xxx-unstable-internal-use-only")] {
118+ if #[cfg(all(feature = "xxx-unstable-internal-use-only", not(target_arch = "wasm32")))] {
119119 fn empty_container(_container: &crate::Element) {}
120120 fn initialize_container(_container: &crate::Element) {}
121121 } else {
@@ -301,33 +301,36 @@ impl VdomInnerExclusive {
301301 }
302302
303303 fn diff<'a>(&mut self, registry: &mut EventsRegistry, old: Node<'a>, new: Node<'a>) {
304− match (&new, old) {
305− (&Node::Text(TextNode { text: new_text }), Node::Text(TextNode { text: old_text })) => {
304+ match (&new.kind, old.kind) {
305+ (
306+ &NodeKind::Text(TextNode { text: new_text }),
307+ NodeKind::Text(TextNode { text: old_text }),
308+ ) => {
306309 debug!(" both are text nodes");
307310 if new_text != old_text {
308311 debug!(" text needs updating");
309312 self.change_list.emit_set_text(new_text);
310313 }
311314 }
312− (&Node::Text(_), Node::Element(_)) => {
315+ (&NodeKind::Text(_), NodeKind::Element(_)) => {
313316 debug!(" replacing a text node with an element");
314317 self.create(registry, new);
315318 self.change_list.emit_replace_with();
316319 }
317− (&Node::Element(_), Node::Text(_)) => {
320+ (&NodeKind::Element(_), NodeKind::Text(_)) => {
318321 debug!(" replacing an element with a text node");
319322 self.create(registry, new);
320323 self.change_list.emit_replace_with();
321324 }
322325 (
323− &Node::Element(ElementNode {
326+ &NodeKind::Element(ElementNode {
324327 tag_name: new_tag_name,
325328 listeners: new_listeners,
326329 attributes: new_attributes,
327330 children: new_children,
328331 namespace: new_namespace,
329332 }),
330− Node::Element(ElementNode {
333+ NodeKind::Element(ElementNode {
331334 tag_name: old_tag_name,
332335 listeners: old_listeners,
333336 attributes: old_attributes,
@@ -477,11 +480,11 @@ impl VdomInnerExclusive {
477480 }
478481
479482 fn create<'a>(&mut self, registry: &mut EventsRegistry, node: Node<'a>) {
480− match node {
481− Node::Text(TextNode { text }) => {
483+ match node.kind {
484+ NodeKind::Text(TextNode { text }) => {
482485 self.change_list.emit_create_text_node(text);
483486 }
484− Node::Element(ElementNode {
487+ NodeKind::Element(ElementNode {
485488 tag_name,
486489 listeners,
487490 attributes,
tests/web/main.rs+16−10
@@ -1,9 +1,9 @@
11//! Test suite for the Web and headless browsers.
22
3−#![cfg(target_arch = "wasm32")]
3+#![cfg(all(feature = "xxx-unstable-internal-use-only", target_arch = "wasm32"))]
44
55use bumpalo::Bump;
6−use dodrio::{Attribute, Node, Render, RenderContext, Vdom};
6+use dodrio::{Attribute, ElementNode, Node, NodeKind, Render, RenderContext, TextNode, Vdom};
77use futures::prelude::*;
88use log::*;
99use std::rc::Rc;
@@ -68,8 +68,8 @@ pub fn assert_rendered<R: Render>(container: &web_sys::Element, r: &R) {
6868 debug!("check_render:");
6969 debug!(" actual = {}", stringify_actual_node(&actual));
7070 debug!(" expected = {:#?}", expected);
71− match expected {
72− Node::Text(text_node) => {
71+ match expected.kind {
72+ NodeKind::Text(TextNode { text }) => {
7373 assert_eq!(
7474 actual.node_name().to_uppercase(),
7575 "#TEXT",
@@ -77,22 +77,28 @@ pub fn assert_rendered<R: Render>(container: &web_sys::Element, r: &R) {
7777 );
7878 assert_eq!(
7979 actual.text_content().unwrap_or_default(),
80− text_node.text(),
80+ text,
8181 "actual.text_content() == expected.text()"
8282 );
8383 }
84− Node::Element(elem) => {
84+ NodeKind::Element(ElementNode {
85+ tag_name,
86+ attributes,
87+ children,
88+ namespace,
89+ ..
90+ }) => {
8591 assert_eq!(
8692 actual.node_name().to_uppercase(),
87− elem.tag_name().to_uppercase(),
93+ tag_name.to_uppercase(),
8894 "actual.node_name() == expected.tag_name()"
8995 );
9096 let actual = actual
9197 .dyn_ref::<web_sys::Element>()
9298 .expect("`actual` should be an `Element`");
93− check_attributes(actual.attributes(), elem.attributes());
94− check_children(actual.child_nodes(), elem.children());
95− if let Some(namespace) = elem.namespace() {
99+ check_attributes(actual.attributes(), attributes);
100+ check_children(actual.child_nodes(), children);
101+ if let Some(namespace) = namespace {
96102 assert_eq!(actual.namespace_uri(), Some(namespace.into()))
97103 }
98104 }