Skip to content

RFC Styletron strategy implemention #1

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

Open
wants to merge 8 commits into
base: modernize
Choose a base branch
from
Open

RFC Styletron strategy implemention #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a501660
add styletron, update deps
chrisirhc May 17, 2020
c97d384
prep showcase for using styletron
chrisirhc May 17, 2020
56259de
first style moved into styletron
chrisirhc May 17, 2020
f8ca481
iteration 2
chrisirhc May 17, 2020
d48aacd
co-locate classname with component but this adds hooks overhead and a…
chrisirhc May 17, 2020
48a34dc
iteration 3 to retain most co-locate the className
chrisirhc May 18, 2020
1ad374f
move into styling-utils
chrisirhc May 18, 2020
018f088
move axis line
chrisirhc May 18, 2020
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
6 changes: 5 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,15 @@
"react-dom": "^16.13.1",
"stylelint": "^7.7.1",
"stylelint-config-standard": "^15.0.1",
"styletron-engine-atomic": "^1.4.6",
"styletron-react": "^5.2.7",
"tape": "^4.6.3",
"uglify-js": "^2.8.22"
},
"peerDependencies": {
"react": "15.3.0 - 16.x"
"react": ">= 16.8.0 < 17",
"react-dom": ">= 16.8.0 < 17",
"styletron-react": "^5.2.2"
},
"keywords": [
"d3",
Expand Down
12 changes: 11 additions & 1 deletion showcase/app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,17 @@
import ReactDOM from 'react-dom';
import React from 'react';
import document from 'global/document';
import {Provider as StyletronProvider, DebugEngine} from "styletron-react";
import {Client as Styletron} from "styletron-engine-atomic";

import {BrowserRouter, Route} from 'react-router-dom';

import ShowcaseApp from './showcase-app';
import '../src/styles/examples.scss';

const debug = new DebugEngine();
const engine = new Styletron();

export default function App(props) {
// using react-router to trigger react updates on url change
return (
Expand All @@ -39,4 +44,9 @@ export default function App(props) {
const el = document.createElement('div');
document.body.appendChild(el);

ReactDOM.render(<App />, el);
ReactDOM.render(
<StyletronProvider value={engine} debug={debug} debugAfterHydration>
<App />
</StyletronProvider>,
el
);
3 changes: 2 additions & 1 deletion src/plot/axis/axis-line.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import React from 'react';
import PropTypes from 'prop-types';

import {ORIENTATION} from 'utils/axis-utils';
import {XYPlotAxisLine} from '../styled-components';

const {LEFT, RIGHT, TOP, BOTTOM} = ORIENTATION;

Expand Down Expand Up @@ -69,7 +70,7 @@ function AxisLine({orientation, width, height, style}) {
};
}
return (
<line {...lineProps} className="rv-xy-plot__axis__line" style={style} />
<XYPlotAxisLine {...lineProps} style={style} />
);
}

Expand Down
4 changes: 2 additions & 2 deletions src/plot/axis/decorative-axis.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import PropTypes from 'prop-types';

import AbstractSeries from 'plot/series/abstract-series';
import DecorativeAxisTicks from './decorative-axis-ticks';
import {XYPlotAxisLine} from '../styled-components';
import Animation from 'animation';
import {getCombinedClassName} from 'utils/styling-utils';

Expand Down Expand Up @@ -78,15 +79,14 @@ class DecorativeAxis extends AbstractSeries {
className={getCombinedClassName(predefinedClassName, className)}
transform={`translate(${marginLeft},${marginTop})`}
>
<line
<XYPlotAxisLine
{...{
x1: x({x: axisStart.x}),
x2: x({x: axisEnd.x}),
y1: y({y: axisStart.y}),
y2: y({y: axisEnd.y}),
...style.line
}}
className="rv-xy-plot__axis__line"
/>
<g className="rv-xy-manipulable-axis__ticks">
{DecorativeAxisTicks({
Expand Down
30 changes: 30 additions & 0 deletions src/plot/styled-components.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/* eslint-disable camelcase */
import {styledWithClass} from 'utils/styling-utils';

const $rv_xy_plot_axis_font_color = '#6b6b76';
const $rv_xy_plot_axis_line_color = '#e6e6e9';
const $rv_xy_plot_axis_font_size = '11px';
const $rv_xy_plot_tooltip_background = '#3a3a48';
const $rv_xy_plot_tooltip_color = '#fff';
const $rv_xy_plot_tooltip_font_size = '12px';
const $rv_xy_plot_tooltip_border_radius = '4px';
const $rv_xy_plot_tooltip_shadow = '0 2px 4px rgba(0, 0, 0, 0.5)';
const $rv_xy_plot_tooltip_padding = '7px 10px';

export const XYPlotInnerSvg = styledWithClass(
'svg',
'rv-xy-plot__inner',
{
display: 'block',
}
);

export const XYPlotAxisLine = styledWithClass(
'line',
'rv-xy-plot__axis__line',
{
fill: 'none',
strokeWidth: '2px',
stroke: $rv_xy_plot_axis_line_color,
}
);
6 changes: 3 additions & 3 deletions src/plot/xy-plot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import PropTypes from 'prop-types';
import equal from 'deep-equal';

import {getCombinedClassName} from 'utils/styling-utils';
import {XYPlotInnerSvg} from './styled-components';

import {
extractScalePropsFromProps,
Expand Down Expand Up @@ -543,8 +544,7 @@ class XYPlot extends React.Component {
}}
className={getCombinedClassName("rv-xy-plot", className)}
>
<svg
className="rv-xy-plot__inner"
<XYPlotInnerSvg
width={width}
height={height}
style={style}
Expand All @@ -562,7 +562,7 @@ class XYPlot extends React.Component {
onWheel={onWheel}
>
{components.filter(c => c && c.type.requiresSVG)}
</svg>
</XYPlotInnerSvg>
{this.renderCanvasComponents(components, this.props)}
{components.filter(c => c && !c.type.requiresSVG && !c.type.isCanvas)}
</div>
Expand Down
10 changes: 0 additions & 10 deletions src/styles/plot.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,6 @@ $rv-xy-plot-tooltip-padding: 7px 10px;
}
}

.rv-xy-plot__inner {
display: block;
}

.rv-xy-plot__axis__line {
fill: none;
stroke-width: 2px;
stroke: $rv-xy-plot-axis-line-color;
}

.rv-xy-plot__axis__tick__line {
stroke: $rv-xy-plot-axis-line-color;
}
Expand Down
9 changes: 9 additions & 0 deletions src/utils/styling-utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.
import React from 'react';
import {styled, withWrapper} from 'styletron-react';

/**
* Generates interpolated class names signature based on multiple class names
Expand All @@ -28,3 +30,10 @@
export function getCombinedClassName(...classNames) {
return classNames.filter(cn => cn && typeof cn === 'string').join(' ')
}

export function styledWithClass(elem, className, styleObj) {
// eslint-disable-next-line react/display-name
return withWrapper(styled(elem, styleObj), Styled => props => (
<Styled className={className} {...props} />
Copy link

@Xiot Xiot May 18, 2020

Choose a reason for hiding this comment

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

It would be interesting to take a deep dive on the performance between hooks and nested components.

This is the component tree using the method that you have above
image

Changing it to use useStyletron with this

export function styledWithClass(elem, definitionClassName, styleObj) {

  const Component = elem;
  const Styled = props => {
    const [css] = useStyletron();
    const {className, style, ...rest} = props;
    const styleClass = css({...styleObj, ...style});
    return <Component className={getCombinedClassName(styleClass, definitionClassName, className)} {...rest} />
  }
  Styled.displayName = `styled(${getDisplayName(elem)})`
  return Styled;
}

we can get the component tree to look like
image

Copy link

@Xiot Xiot May 18, 2020

Choose a reason for hiding this comment

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

as a point of reference, these are the hooks that are being used
image

));
}
80 changes: 80 additions & 0 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@
lodash "^4.17.5"
to-fast-properties "^2.0.0"

"@rtsao/csstype@2.6.5-forked.0":
version "2.6.5-forked.0"
resolved "https://registry.yarnpkg.com/@rtsao/csstype/-/csstype-2.6.5-forked.0.tgz#b5b4e2a07ad83a91874ebf5fabdb73dc8c1632f6"
integrity sha512-0HwnY8uPWcCloTgdbbaJG3MbDUfNf6yKWZfCKxFv9yj2Sbp4mSKaIjC7Cr/5L4hMxvrrk85CU3wlAg7EtBBJ1Q==

JSONStream@^0.8.4:
version "0.8.4"
resolved "https://registry.npmjs.org/JSONStream/-/JSONStream-0.8.4.tgz#91657dfe6ff857483066132b4618b62e8f4887bd"
Expand Down Expand Up @@ -1704,6 +1709,14 @@ css-color-names@0.0.3:
version "0.0.3"
resolved "https://registry.npmjs.org/css-color-names/-/css-color-names-0.0.3.tgz#de0cef16f4d8aa8222a320d5b6d7e9bbada7b9f6"

css-in-js-utils@^2.0.0:
version "2.0.1"
resolved "https://registry.yarnpkg.com/css-in-js-utils/-/css-in-js-utils-2.0.1.tgz#3b472b398787291b47cfe3e44fecfdd9e914ba99"
integrity sha512-PJF0SpJT+WdbVVt0AOYp9C8GnuruRlL/UFW7932nLWmFLQTaWEzTBQEx7/hn4BuV+WON75iAViSUJLiU3PKbpA==
dependencies:
hyphenate-style-name "^1.0.2"
isobject "^3.0.1"

css-rule-stream@^1.1.0:
version "1.1.0"
resolved "https://registry.npmjs.org/css-rule-stream/-/css-rule-stream-1.1.0.tgz#3786e7198983d965a26e31957e09078cbb7705a2"
Expand Down Expand Up @@ -2984,6 +2997,11 @@ husky@^1.1.2:
run-node "^1.0.0"
slash "^2.0.0"

hyphenate-style-name@^1.0.2:
version "1.0.3"
resolved "https://registry.yarnpkg.com/hyphenate-style-name/-/hyphenate-style-name-1.0.3.tgz#097bb7fa0b8f1a9cf0bd5c734cf95899981a9b48"
integrity sha512-EcuixamT82oplpoJ2XU4pDtKGWQ7b00CD9f1ug9IaQ3p1bkHMiKCZ9ut9QDI6qsa6cpUuB+A/I+zLtdNK4n2DQ==

iconv-lite@0.4.13, iconv-lite@~0.4.13:
version "0.4.13"
resolved "https://registry.npmjs.org/iconv-lite/-/iconv-lite-0.4.13.tgz#1f88aba4ab0b1508e8312acc39345f36e992e2f2"
Expand Down Expand Up @@ -3057,6 +3075,13 @@ inline-source-map@~0.6.0:
dependencies:
source-map "~0.5.3"

inline-style-prefixer@^5.1.0:
version "5.1.2"
resolved "https://registry.yarnpkg.com/inline-style-prefixer/-/inline-style-prefixer-5.1.2.tgz#e5a5a3515e25600e016b71e39138971228486c33"
integrity sha512-PYUF+94gDfhy+LsQxM0g3d6Hge4l1pAqOSOiZuHWzMvQEGsbRQ/ck2WioLqrY2ZkHyPgVUXxn+hrkF7D6QUGbA==
dependencies:
css-in-js-utils "^2.0.0"

inquirer@^0.12.0:
version "0.12.0"
resolved "https://registry.npmjs.org/inquirer/-/inquirer-0.12.0.tgz#1ef2bfd63504df0bc75785fff8c2c41df12f077e"
Expand Down Expand Up @@ -3289,6 +3314,11 @@ isobject@^2.0.0:
dependencies:
isarray "1.0.0"

isobject@^3.0.1:
version "3.0.1"
resolved "https://registry.yarnpkg.com/isobject/-/isobject-3.0.1.tgz#4e431e92b11a9731636aa1f9c8d1ccbcfdab78df"
integrity sha1-TkMekrEalzFjaqH5yNHMvP2reN8=

isomorphic-fetch@^2.1.1:
version "2.2.1"
resolved "https://registry.npmjs.org/isomorphic-fetch/-/isomorphic-fetch-2.2.1.tgz#611ae1acf14f5e81f729507472819fe9733558a9"
Expand Down Expand Up @@ -3364,6 +3394,11 @@ js-tokens@^3.0.0:
version "3.0.1"
resolved "https://registry.npmjs.org/js-tokens/-/js-tokens-3.0.1.tgz#08e9f132484a2c45a30907e9dc4d5567b7f114d7"

"js-tokens@^3.0.0 || ^4.0.0":
version "4.0.0"
resolved "https://registry.yarnpkg.com/js-tokens/-/js-tokens-4.0.0.tgz#19203fb59991df98e3a287050d4647cdeaf32499"
integrity sha512-RdJUflcE3cUzKiMqQgsCu06FPu9UdIJO0beYbPhHN4k6apgJtifcoCtT9bcxOpYBtpD2kCM6Sbzg4CausW/PKQ==

js-yaml@^3.4.3, js-yaml@^3.5.1:
version "3.8.4"
resolved "https://registry.npmjs.org/js-yaml/-/js-yaml-3.8.4.tgz#520b4564f86573ba96662af85a8cafa7b4b5a6f6"
Expand Down Expand Up @@ -3671,6 +3706,13 @@ loose-envify@^1.0.0, loose-envify@^1.1.0, loose-envify@^1.3.1:
dependencies:
js-tokens "^3.0.0"

loose-envify@^1.4.0:
version "1.4.0"
resolved "https://registry.yarnpkg.com/loose-envify/-/loose-envify-1.4.0.tgz#71ee51fa7be4caec1a63839f7e682d8132d30caf"
integrity sha512-lyuxPGr/Wfhrlem2CL/UcnUc1zcqKAImBDzukY7Y5F/yQiNdko6+fRLevlw1HgMySw7f611UIY408EtxRSoK3Q==
dependencies:
js-tokens "^3.0.0 || ^4.0.0"

loud-rejection@^1.0.0:
version "1.6.0"
resolved "https://registry.npmjs.org/loud-rejection/-/loud-rejection-1.6.0.tgz#5b46f80147edee578870f086d04821cf998e551f"
Expand Down Expand Up @@ -4516,6 +4558,15 @@ prop-types@^15.5.4, prop-types@^15.5.8:
fbjs "^0.8.9"
loose-envify "^1.3.1"

prop-types@^15.6.0:
version "15.7.2"
resolved "https://registry.yarnpkg.com/prop-types/-/prop-types-15.7.2.tgz#52c41e75b8c87e72b9d9360e0206b99dcbffa6c5"
integrity sha512-8QQikdH7//R2vurIJSutZ1smHYTcLpRWEOlHnzcWHmBYrOGUysKwSsrC89BCiFj3CbrfJ/nXFdJepOVrY1GCHQ==
dependencies:
loose-envify "^1.4.0"
object-assign "^4.1.1"
react-is "^16.8.1"

prop-types@^15.6.2:
version "15.6.2"
resolved "https://registry.yarnpkg.com/prop-types/-/prop-types-15.6.2.tgz#05d5ca77b4453e985d60fc7ff8c859094a497102"
Expand Down Expand Up @@ -4611,6 +4662,11 @@ react-dom@^16.13.1:
prop-types "^15.6.2"
scheduler "^0.19.1"

react-is@^16.8.1:
version "16.13.1"
resolved "https://registry.yarnpkg.com/react-is/-/react-is-16.13.1.tgz#789729a4dc36de2999dc156dd6c1d9c18cea56a4"
integrity sha512-24e6ynE2H+OKt4kqsOvNd8kBpV65zoxbA4BVsEOB3ARVWQki/DHzaUoC5KuON/BiccDaCCTZBuOcfZs70kR8bQ==

react-motion@^0.5.2:
version "0.5.2"
resolved "https://registry.npmjs.org/react-motion/-/react-motion-0.5.2.tgz#0dd3a69e411316567927917c6626551ba0607316"
Expand Down Expand Up @@ -5386,6 +5442,30 @@ stylelint@^7.7.1:
svg-tags "^1.0.0"
table "^4.0.1"

styletron-engine-atomic@^1.4.6:
version "1.4.6"
resolved "https://registry.yarnpkg.com/styletron-engine-atomic/-/styletron-engine-atomic-1.4.6.tgz#0cfaa32dd8c386a4736b9cd25361711712c4fd98"
integrity sha512-1dPXjIjdd3nrQdh1PYvQ3MOd0syzAkFVJzaH31rwy0lhSSsKzfCqn1WLtaLqvL5z89J9wwAsu9CaNZis06K+4g==
dependencies:
inline-style-prefixer "^5.1.0"
styletron-standard "^3.0.4"

styletron-react@^5.2.7:
version "5.2.7"
resolved "https://registry.yarnpkg.com/styletron-react/-/styletron-react-5.2.7.tgz#7aa283a0b06787024f96289480c60bafed5048cf"
integrity sha512-PUUf7MAv9aaO8oT0B4ryPreeLi3BT1kQ7WIDNhkkpLkvCzk+pOvmtaZJJZRIxWPimj8756rLZqpF7/2ssF9KhQ==
dependencies:
prop-types "^15.6.0"
styletron-standard "^3.0.4"

styletron-standard@^3.0.4:
version "3.0.4"
resolved "https://registry.yarnpkg.com/styletron-standard/-/styletron-standard-3.0.4.tgz#2b286752f464e4c9824faaee466013aa8f13cf6b"
integrity sha512-AozOYUEtB5m0s+6ieLCObqYsBitLJmag3IoD/yg0Y9X32AYKwtGO4emx9cljD25k2S/R1XmLPgThOU+orjXTKg==
dependencies:
"@rtsao/csstype" "2.6.5-forked.0"
inline-style-prefixer "^5.1.0"

subarg@^1.0.0:
version "1.0.0"
resolved "https://registry.npmjs.org/subarg/-/subarg-1.0.0.tgz#f62cf17581e996b48fc965699f54c06ae268b8d2"
Expand Down