perf: Improve code splitting

[bprusinowski]

• Do not import ContentMDXProvider in _app.tsx, to prevent adding ~2.8MB in payload size to every page, but rather only use it where it's necessary

Further ideas

Right now, if we want to render a simple column chart, we bundle all column chart related logic, including the grouped chart state, stacked chart state, code for rendering grouped & stacked charts, etc.

I believe that this could be avoided if we split the code in a manner that we treat these as different chart types and not include them in one function (ChartColumnsVisualization) that conditionally renders a given chart subtype. I think this would be a bigger change, as we would need to treat these chart subtypes as chart types, e.g. "column-simple", "column-grouped", "column-stacked". With this we wouldn't bundle things that are not necessary.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
visualization-tool	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 20, 2023 8:05am

[bprusinowski]

@ptbrowne I am going to merge, let me know if you have any thoughts on this :)

[ptbrowne]

Thanks Bartosz, LGTM.

On the further notes, I am wondering why every chart is bundled when the dynamic tag is used (I see that in app/components/common-chart.tsx) ? It would have seemed to me that the components wrapped in dynamic would only bring their code when the component was rendered ?

[bprusinowski]

Yes, that's the case – but the column chart that we import is basically this,

visualization-tool/app/charts/column/chart-column.tsx

Lines 95 to 217 in 91b2d00

	export const ChartColumns = memo(
	({
	observations,
	dimensions,
	measures,
	chartConfig,
	}: {
	observations: Observation[];
	dimensions: DimensionMetadataFragment[];
	measures: DimensionMetadataFragment[];
	chartConfig: ColumnConfig;
	}) => {
	const { fields, interactiveFiltersConfig } = chartConfig;
	return (
	<>
	{/* FIXME: These checks should probably be handled somewhere else */}
	{fields.segment?.componentIri && fields.segment.type === "stacked" ? (
	<StackedColumnsChart
	data={observations}
	fields={fields}
	dimensions={dimensions}
	measures={measures}
	aspectRatio={0.4}
	chartConfig={chartConfig}
	>
	<ChartContainer>
	<ChartSvg>
	<AxisHeightLinear /> <AxisWidthBand />
	<ColumnsStacked /> <AxisWidthBandDomain />
	<InteractionColumns />
	{interactiveFiltersConfig?.timeRange.active && <BrushTime />}
	</ChartSvg>
	<Tooltip type="multiple" />
	</ChartContainer>
	<LegendColor
	symbol="square"
	interactive={
	fields.segment && interactiveFiltersConfig?.legend.active
	}
	/>
	{fields.animation && (
	<TimeSlider
	componentIri={fields.animation.componentIri}
	dimensions={dimensions}
	showPlayButton={fields.animation.showPlayButton}
	animationDuration={fields.animation.duration}
	animationType={fields.animation.type}
	/>
	)}
	</StackedColumnsChart>
	) : fields.segment?.componentIri &&
	fields.segment.type === "grouped" ? (
	<GroupedColumnChart
	data={observations}
	fields={fields}
	dimensions={dimensions}
	measures={measures}
	aspectRatio={0.4}
	chartConfig={chartConfig}
	>
	<ChartContainer>
	<ChartSvg>
	<AxisHeightLinear />
	<AxisWidthBand />
	<ColumnsGrouped />
	<ErrorWhiskersGrouped />
	<AxisWidthBandDomain />
	<InteractionColumns />
	{interactiveFiltersConfig?.timeRange.active && <BrushTime />}
	</ChartSvg>
	<Tooltip type="multiple" />
	</ChartContainer>
	<LegendColor
	symbol="square"
	interactive={
	fields.segment && interactiveFiltersConfig?.legend.active
	}
	/>
	{fields.animation && (
	<TimeSlider
	componentIri={fields.animation.componentIri}
	dimensions={dimensions}
	showPlayButton={fields.animation.showPlayButton}
	animationDuration={fields.animation.duration}
	animationType={fields.animation.type}
	/>
	)}
	</GroupedColumnChart>
	) : (
	<ColumnChart
	data={observations}
	measures={measures}
	dimensions={dimensions}
	aspectRatio={0.4}
	chartConfig={chartConfig}
	>
	<ChartContainer>
	<ChartSvg>
	<AxisHeightLinear />
	<AxisWidthBand />
	<Columns />
	<ErrorWhiskers />
	<AxisWidthBandDomain />
	<InteractionColumns />
	{interactiveFiltersConfig?.timeRange.active && <BrushTime />}
	</ChartSvg>
	<Tooltip type="single" />
	</ChartContainer>
	{fields.animation && (
	<TimeSlider
	componentIri={fields.animation.componentIri}
	dimensions={dimensions}
	showPlayButton={fields.animation.showPlayButton}
	animationDuration={fields.animation.duration}
	animationType={fields.animation.type}
	/>
	)}
	</ColumnChart>
	)}
	</>
	);
	}
	);

which means that we have three charts inside one function.

[bprusinowski]

I think the problem is that we have one function that renders different chart subtype based on some conditions – if it was split inside three functions, that would prevent bundling everything. I tried to split that, but still there was a root check on chart type (column)

visualization-tool/app/components/common-chart.tsx

Lines 68 to 89 in 91b2d00

	switch (chartConfig.chartType) {
	case "column":
	return <ChartColumnsVisualization {...props} chartConfig={chartConfig} />;
	case "line":
	return <ChartLinesVisualization {...props} chartConfig={chartConfig} />;
	case "area":
	return <ChartAreasVisualization {...props} chartConfig={chartConfig} />;
	case "scatterplot":
	return (
	<ChartScatterplotVisualization {...props} chartConfig={chartConfig} />
	);
	case "pie":
	return <ChartPieVisualization {...props} chartConfig={chartConfig} />;
	case "table":
	return <ChartTableVisualization {...props} chartConfig={chartConfig} />;
	case "map":
	return <ChartMapVisualization {...props} chartConfig={chartConfig} />;
	default:
	const _exhaustiveCheck: never = chartConfig;
	return _exhaustiveCheck;
	}
	};

+ additional checks below it on which subtype to render, and that didn't work, that's why I think these would need to be distinguishable at the same level as other dynamic imports (chart type: area, line, ...) 🤔 I hope it makes sense 😅

[ptbrowne]

I see. I think it would be good to use a bundle analyser (like webpack bundle analyser) to see if effectively this is here that we have the biggest size. When I have a quick check at the column charts file, I see that they do not weigh a lot. I would see bundle size being more a result of libraries (or maybe icons) that we import when not necessary, more than application code. ``` ➜ cd app/charts/column ➜ ls (base) chart-column.tsx columns-grouped.tsx columns-stacked-state.tsx columns-state.tsx overlay-columns.tsx columns-grouped-state.tsx columns-simple.tsx columns-stacked.tsx constants.ts rendering-utils.tsx ➜ du -sh . (base) 80K . ➜ cat * | gzip | wc -c (base) 9193 ``` On 20 June 2023 at 10:33:46, Bartosz Prusinowski ***@***.***) wrote: Yes, that's the case – but the column chart that we import is basically this, https://github.com/visualize-admin/visualization-tool/blob/91b2d00e38067535f2b9b15bb6a645621a6a54dc/app/charts/column/chart-column.tsx#L95-L217 which means that we have three charts inside one function. — Reply to this email directly, view it on GitHub <#1071 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADRVLWCX5HYGNL3JMZ3ZVDXMFN6VANCNFSM6AAAAAAZL74O74> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[bprusinowski]

Yup, I used @next/bundle-analyzer and found the issue with MDX provider 😅 But it's true that icons are the next big thing to look at 👍