Closed jeremy-coleman closed 1 year ago
252kb gzippd app , in all its glory
@kenotron this would be right about in your garden 😄
@dzearing @cliffkoh FYI
@jeremy-coleman We need more info on your repro. I assure you we don't use jQuery. :)
For example, I just ran create-react-app (which now has webpack 4), and did what you did:
I believe the first import extra bytes are due to scss (side effects files) not getting tree shaken.
Do you have a webpack-bundle-analyzer graph of your build?
I also compared actual file build output:
Without any references, raw create-react-app:
34.71 KB build/static/js/1.fa92c112.chunk.js 763 B build/static/js/runtime~main.229c360f.js 716 B build/static/js/main.afe6f131.chunk.js 512 B build/static/css/main.7875265d.chunk.css
With mergeStyleSets
usage from 'office-ui-fabric-react'
import:
47.85 KB build/static/js/1.44535d0f.chunk.js 763 B build/static/js/runtime~main.229c360f.js 749 B build/static/js/main.202e43a5.chunk.js 511 B build/static/css/main.3d194e55.chunk.css
Same, but using '@uifabric/merge-styles'
import, avoiding importing 9k of scss side effects:
38.1 KB (-9.75 KB) build/static/js/1.2b8e88d7.chunk.js 763 B build/static/js/runtime~main.229c360f.js 751 B (+2 B) build/static/js/main.7a66b132.chunk.js 510 B (-1 B) build/static/css/main.e7ec6a68.chunk.css
Can you also clarify what you'd like to fix? Fixes always welcome but if you provide more information, maybe we can offer more feedback on if the approach makes sense to us as well.
the screen shots i posted were just from that single file, the only difference being react-scripts-ts version
I was comparing a few actually,roughly formatted as library(total) 35kb base mobx - 20kb gzip (55kb) rxjs 10kb (46kb) undux 12kb (42kb) typestyle 4kb(39kb) redux / react-connect 6kb(41kb) @emotion/styled - 9kb(44kb) glamor-jss +20kb wtf @material/styles (using styled only, no actual styles) - 16kb(51kb) 4kb(39kb) for styled, createComponent, and mergeStylesets from the @uifabric libs 255kb office ui
I have inspected bundles several times, but the deps are too nested and cross-repo to do anything about it (which is why merging them all together will reduce size significantly)
i'd like to do the following:
Hmm. 255k still sounds really way way off for the entire library. Are you using the latest webpack (4 or greater)? If you import a single component webpack 4 should tree shake out unused non side-effect things.
Can you share a project? It would be extremely helpful.
I am not sure what you mean "same patch" and how that would affect bundle size.
Path based imports are a bad idea, as they mean that the actual file paths become a contract. You can, today, import individual components from /lib/TopLevelImport
to reduce the graph if you're not using webpack 4. But I highly recommend using webpack 4 or greater.
Rollup is an es6 module concatenation tool. I don't think you will see improvements in bundle size here.
FWIW, using current create-react-app, i modified the root index file and did this:
export * from 'office-ui-fabric-react';
So exporting the entire library, every single component, would be ~252 KB (minus 34.71 KB) = 218k total.
252.86 KB (+205.01 KB) build/static/js/1.687d7d43.chunk.js 4.57 KB (+3.83 KB) build/static/js/main.d2c5082f.chunk.js 763 B build/static/js/runtime~main.229c360f.js 510 B (-1 B) build/static/css/main.dd4a0960.chunk.css
We highly recommend not doing this and instead using webpack 4 to use tree shaking to reduce the bundle size to only what you need.
In addition, we are looking at breaking up oufr into smaller packages. Package names are a fine contract to import, and we want to give people the option to take advantage of more granular versioning boundaries.
I also have a breakdown of that 218k of what that includes:
https://codepen.io/dzearing/full/XoPRxg
The numbers are estimates and module concatenation optimization is disabled so that you can see the module impact.
@jeremy-coleman mentioned to move to rollup - perhaps it would be easier to upgrade webpack rather than switch to another bundler?
I should have clarified my rollup statement above; rollup will also tree shake out just what you want, but yeah, Webpack on the consumer side will do the same thing but with more configurability on what gets output.
imo rollup is better for consumable libs , especially ones written in typescript, because the output is readable in a nice single file with all of your named exports. also webpack doesn't do so well with export * which is everywhere in all the repos
also since you're using rush / pnpm , you likely won't have duplicate bundles , but that doesn't mean yarn and npm consumers wont - thats why putting all libs on the same version number is beneficial, you can be certain you're not importing the same library twice.
import autoExternal from 'rollup-plugin-auto-external' import typescript from 'rollup-plugin-typescript'
import {terser} from 'rollup-plugin-terser'
var pkgJson = require('./package.json') var devDeps = Object.keys(pkgJson.devDependencies || {}) var peerDeps = Object.keys(pkgJson.peerDependencies || {}) var pkgDeps = Object.keys(pkgJson.dependencies || {})
export default {
external: ['react'],
input: 'src/index.ts',
output: [
{file: lib/index.js
, format: "es", sourcemap: false},
{file: lib/index.mjs
, format: "es", sourcemap: false},
{file: lib/index.cjs.js
, format: "cjs", sourcemap: false},
],
plugins: [
autoExternal(),
typescript(),
//terser() for min,
],
}
//ts config
// { // "compilerOptions": { // "rootDir": "./src", // "outDir": "./lib", // "baseUrl": "./src", // "noImplicitAny": false, // "moduleResolution": "node", // "declaration": true, // "target": "esnext", // "module": "esnext", // "allowJs": false, // "jsx": "react", // "experimentalDecorators": true, // "lib": ["dom","scripthost","esnext"], // "skipLibCheck": true // }, // "include": ["src"], // "exclude": ["node_modules"] // }
//npm tasks
// "build": "yarn bundle:code && yarn bundle:dts",
// "bundle:code":"rollup -c rollup.pkg.js",
// "bundle:dts":"dts-bundle-generator -o lib/index.d.ts src/index.ts",
//deps
// "typescript": "^3.2.2",
// "rollup":"1.0.2",
// "rollup-plugin-typescript":"1.0.0",
// "rollup-plugin-auto-external":"2.0.0",
// "dts-bundle-generator":"2.0.0",
// "rollup-plugin-terser":"4.0.1",
// "tslib":"1.9.3"
that'll get u setup with rollup if you want to see the difference in output quality
Your project is using webpack 3, as it was created with an old create-react-app. If you use the latest create-react-app to rescaffold, you'll be able to see significantly different results.
npm init react-app mui-bundle-size
Also thank you for the config for rollup, I'll give it a try. The last time I used it was back in 2016 when it would throw exceptions when hitting certain typescript statements.
are you sure?that's the vnext of react-scripts-ts (has webpack dependency of 4.19.1 in its package.json) the bundled 3.x is workbox (the worker generator plugin)
oh also a single repo lets you find stuff like this easily
threw this together , bedtime now. bundles foundation, merge stylese, set-version, styling, utilities, and variants, prob around 15k gzipped for all, and a single .d.ts for easy type reworking
will get the main repo bundled tomorrow
https://github.com/jeremy-coleman/mui-fab-demo/tree/office-ui-repo-work-sample
what is the mergeStyles equivalent of
.rootIsCompact :global(.ms-DocumentCard-details) {
display: flex;
flex-direction: column;
flex: 1;
justify-content: space-between;
overflow: hidden; }
import { mergeStyles } from "@uifabric/merge-styles";
export const rootIsCompact = mergeStyles({
selectors: {
"&:global": {
display: "flex",
flexDirection: "column",
flex: "1",
justifyContent: "space-between",
overflow: "hidden"
}
}
});
export const rootIsCompact = mergeStyles({
selectors: {
":global": {
display: "flex",
flexDirection: "column",
flex: "1",
justifyContent: "space-between",
overflow: "hidden"
}
}
});
export const rootIsCompact = mergeStyles({
selectors: {
":global(.ms-DocumentCard-details)": {
display: "flex",
flexDirection: "column",
flex: "1",
justifyContent: "space-between",
overflow: "hidden"
}
}
});
https://codepen.io/dzearing/pen/WLaxap
the selector ".ms-DocumentCard-details" does not need to be wrapped in a :global as it's already a global selector.
mergeStyles({
selectors: {
'& .ms-DocumentCard-details': {
display: 'flex',
flexDirection: 'column',
flex: 1,
justifyContent: 'space-between',
overflow: 'hidden'
}
}
})
so im using a script to convert the existing scss to mergeStyles, and using the document card styles as a test
put it up here https://github.com/jeremy-coleman/ui-fabric-style-converter
just type gulp convert --overwrite in your shell and look in src/styletest
you can also uncomment lines 59/60 to write the ast to a json file
looking for guidance on what the expected output should be to get the current stuff converted, doesn't have to be perfect , can fix small stuff manually
a few other questions, for files with imports like
import {SomethingFromUIFabric} from '../../../utilities'
then in utilities its just exporting everything from @uifabric/package is there a reason for that and are you opposed to changing it to @uifabric/package name?
i've also ran into a few issues with the types , which I think is mosly from legacy typescript versions. Any guidance/thoughts on improving them? Most can be replaced using return types and identity functions now that'll give the same type and reduce possibility of human error AND give better user feed back in the editor - which is the whole point of typing styles anyway right? IE:
to
you can maintain the interface contract as follows:
i think that would ease a migration to CSSTypes if you wanted
anyway, my main question is on the imports for a potential pull request. If I refactor the imports to use '@uifabric/package', is that acceptable?
Couple things;
The scss converter looks pretty awesome! I'm interested in how this works out. Minor note we have DocumentCard conversion out in PR.
The reasoning for the path import was simply that we wanted to keep all package imports in one place. I believe we are actually in a good place now to just import packages. @kenotron as an fyi
(There's a lot (years) of history in import formatting and a lot of the weirdness stems from trying to support AMD.) Now that pretty much everyone is using webpack, and partners which do use AMD already have package mappings set up (AMD does not read your package.json for a "module" or "main" entry, so you end up having to translate what @uifabric/utilities
resolves to manually in AMD config) I think just importing from a package import directly makes sense.
Will talk with ken today about any concerns in wholesale just importing directly from package.
I got a local version mostly working over the weekend, ~680kb non gzip ( no icons, scss or load themes) but includes all of ouif index exports, and all the @uifabric packages
So about half the size of the current one
Also everything is exported as esnext module from a single file for exact usage tree shaking
https://github.com/jeremy-coleman/ui-fabric-style-converter/tree/bundles , mostly at a stopping point , need some guidance on format / output etc. try running the 'gendts' script also, it's great. the .d.ts file and the lib/debug.js file are useful for debugging/ optimizations
IE: in the index.d.ts
and this from lib/debug.js is the named exports of all the bundles. in this picture I am hovering the 'createRef$1', as duplicate (named not code) exports will be have a $(number) appended - you can use this to pinpoint issues that usually stem from import resolution.
also just as a sidebar, i've been experimenting with css props instead of getTheme() or a theme context provider
IE:
let ButtonCSS = mergeStyleSets({
root: {
background: 'var(--themePrimary)'
}
})
you can even create auto color scaling themes with
let ButtonCSS = mergeStyleSets({
root: {
background: 'var(--themePrimary)',
filter: 'brightness(3)'
}
})
and it nests just fine, here's a small 1 pager
let setPrimaryColor = async (color) => void await document.body.style.setProperty("--primary", color);
let setPrimaryThemeVariables = async (color) => void await document.body.style.setProperty("--primary", color);
let fromSelector = async (elementQuery, color) => void await document.querySelector<any>(`${elementQuery}`).style.setProperty("--primary", color);
let pickRandomColor = () => "#"+((1<<24)*Math.random()|0).toString(16)
const ThemeButton = () => (
<button onClick={() => {setPrimaryThemeVariables(pickRandomColor())}}>
body theme
</button>
)
const ThemeButtonInner = () => (
<button onClick={() => {fromSelector('#main-content-body-child', pickRandomColor())}}>
click me
</button>
)
const ThemeButtonTwo = () => (<button onClick={() => {setPrimaryColor('green')}}>set green</button>)
let varStyle = {
backgroundColor: 'var(--primary)'
}
let pagecss = {
padding: '16px 32px',
minHeight: '100%',
width: '100%',
}
let flexfull = {
flex: '1 1 auto'
}
export let AboutPage = (props) => (
<div style={Object.assign(pagecss, varStyle)}>
<div style={{alignContent: 'stretch'}}>
Header
<ThemeButton/>
<ThemeButtonTwo/>
<ThemeButtonInner/>
</div>
<div style={flexfull}>
Body
<div style={varStyle} id='main-content-body-child'>
using same variable as parent (var(--primary))
</div>
</div>
</div>
)
--theme-darker: #004578;
--theme-dark: #005a9e;
--theme-dark-alt: #106ebe;
--theme-secondary: #2b88d8;
--theme-tertiary: #71afe5;
--theme-light: #c7e0f4;
--theme-lighter: #deecf9;
--theme-lighter-alt: #eff6fc;
--black: #000000;
--black-translucent40: rgba(0,0,0,.4);
--neutral-dark: #212121;
--neutral-primary: #333333;
--neutral-primary-alt: #3c3c3c;
--neutral-secondary: #666666;
--neutral-secondary-alt: #767676;
--neutral-tertiary: #a6a6a6;
--neutral-tertiary-alt: #c8c8c8;
--neutral-quaternary: #d0d0d0;
--neutral-quaternary-alt: #dadada;
--neutral-light: #eaeaea;
--neutral-lighter: #f4f4f4;
--neutral-lighter-alt: #f8f8f8;
--accent: #F44336;
--white: #ffffff;
--white-translucent40: rgba(255,255,255,.4);
--orange: #d83b01;
--redDark:#a80000;
or (they are case sensitive)
--themePrimary: #424242;
--themeDarker: #004578;
--themeDark: #005a9e;
--themeDarkAlt: #106ebe;
--themeSecondary: #2b88d8;
--themeTertiary: #71afe5;
--themeLight: #c7e0f4;
--themeLighter: #deecf9;
--themeLighterAlt: #eff6fc;
--black: #000000;
--blackTranslucent40: rgba(0,0,0,.4);
--neutralDark: #212121;
--neutralPrimary: #333333;
--neutralPrimaryAlt: #3c3c3c;
--neutralSecondary: #666666;
--neutralSecondaryAlt: #767676;
--neutralTertiary: #a6a6a6;
--neutralTertiaryAlt: #c8c8c8;
--neutralQuaternary: #d0d0d0;
--neutralQuaternaryAlt: #dadada;
--neutralLight: #eaeaea;
--neutralLighter: #f4f4f4;
--neutralLighterAlt: #f8f8f8;
--accent: #F44336;
--white: #ffffff;
--whiteTranslucent40: rgba(255,255,255,.4);
--orange: #d83b01;
FWIW the earlier mention of webpack 3:
I git cloned your repo I npm installed cd node_modules/webpack cat package.json and look at version
That's what I was referring to.
I am a big fan of css variables, they are awesome. I just am not sure how they will work for IE11...which, unfortunately, we need to support until majority of enterprises move to something else. :(
@JasonGore has checked in a ThemeProvider
component for programatically providing a theme. We wanted to have a layer on top of Customizer
to both manage theme injection but processing as well. Possible we could make extension points here to register css variables as well, so if IE11 isn't a concern for a developer we could provide values through variables. Just an idea.
dts-bundle-generator
is pretty cool! We have been using @microsoft/api-extractor
to do something similar. (https://github.com/Microsoft/web-build-tools/wiki/API-Extractor-~-Creating-.d.ts-rollups) It does a superset of things (like break the build when people remove an export.)
Hi @jeremy-coleman! Sean from the webpack team here. Our guidance to libraries especially with multiple packages that can be consumed similar to office-ui-fabric-react is to not rollup
their ESM code. This would result in a less tree-shakable dependency (especially if you are using webpack 4>).
If you are not yet on webpack 4, I would heavily suggest that you do so (if performance [based on your initial issue submission, yes] is your primary concern).
Luckily being also MSFT I have spent some considerable time with @dzearing and @kenotron to ensure that these packages can be deployed in the most optimal manner (which they are currently). Happy to assist further. ❤️👍
@jeremy-coleman - tried to clone & build this: https://github.com/jeremy-coleman/mui-fab-demo/tree/mui-bundle-size but it won't build... not sure if it was working at some point?
@kenotron https://github.com/jeremy-coleman/ui-fabric-style-converter/tree/bundles is where the most recent work is
@TheLarkInn please have a look at https://github.com/jeremy-coleman/ui-fabric-style-converter/tree/bundles/lib - two things to note. *updated to https://github.com/jeremy-coleman/ui-fabric-style-converter/tree/webpack
1) the rollup bundle is ~200kb smaller compared to the webpack version (the -wp.js file) 2) The non-minified rollup output in debug.js - can webpack produce anything similar?
added comparisons for several implementations at a new branch under https://github.com/jeremy-coleman/ui-fabric-style-converter/tree/webpack
a couple points 1) The normal webpack bundle in that branch is 874kb compared to ouif.min.js v 6.125 of 866kb , so pretty much the same. 2) Oddly , I changed 1 seemingly inconsequential line in the terser config and bundle size dropped to ~500kb(with webpack), changing the mangle property to true instead of blank INCREASES bundle size by ~60% somehow. 3) the rollup and the terser standalone are 470kb, and also probably the best long-term option because you can mod the AST directly for future codemodding. 4) It's unclear what is safe to mangle, but intuitively and just as a norm, you should expect about a 1/4th drop in size from uglifying whereas the base-line webpack only cuts about half. But that is just another reason it's important to NOT use webpack imo , at least not during development.
anyway, bundle size was never the point of my request, it's primarily to fix the path resolutions and maintenance, while also being especially conducive to repo-wide code-mods where you can actually see and immediately test/lint. For example, i tried to pull out the ContextualCommandMenu(bc it was adding 100kb basically an interface). oh what a failed journey that was.
that's the only external imports in the entire repo, pretty amazing for a ~40k loc base
@dzearing I think it's pretty usable now, probably just comes down to user's willingness to use it lol.
js loadThemes({ themePrimary: '(var--(themePrimary)', etc..})
is all it takes or maybe something like getTheme().palette map((k, v) => setCustom(--${k}: ${v}
)). I put together a dashboard type app just to see how it would go doing that, It's really light weight but I definately missed the benefits of ts editor feedback.
@dzearing you might be interested in this, i mentioned it in my style request a few months ago as well
//will allso need to install '@emotion/core'
import styled , { CreateStyled } from '@emotion/styled'
import {IPalette} from 'office-ui-fabric-react'
export default styled as CreateStyled<IPalette>
//renaming here just to show in a single file, typically just re-export styled from your project root
let myStyled = styled as CreateStyled<IPalette>
const Button = myStyled('button')`
padding: 20px;
background-color: ${props => props.theme.themeDarker};
color: ${props => props.theme.notDefined};
border-radius: 3px;
`;
const ButtonObjectStyles = myStyled('button')(({theme}) => ({
padding: '20px',
backgroundColor: theme.themeDarker,
color: theme.notDefined,
borderRadius: 3,
}))
editor:
I think you are missing the point here. This lib isn't meant to be bundled. It's less efficient to do so. The format currently is optimized for an app (bundled with webpack 4+) consuming this lib.
What you've showed is applying tools against the lib itself which accomplishes more bloat vs. less.
I'm seeing potentially similar stuff to @jeremy-coleman regarding production bundle size where the uifabric stuff is amounting to 330k for what I feel is a very simple app and was looking to see what I might be doing wrong. My repo is here and there's a stats.json on there so you can see the breakdown: https://github.com/xrm-rangers/xrm-react
You are inadvertently telling babel to use preset-env
, however you are not setting modules: false
in the babelrc.
Per the babel documentation:
defaults to "auto".
This means that any import
syntax that you are using in your code will be transpiled to use CommonJS syntax instead.
I would encourage you to run your build but using the --display-optimization-bailout
flag (meant to identify why webpack cannot treeshake or scope hoist) and observe the results:
What you will see is that webpack will not scope hoist or tree shake CJS/AMD/Require module syntax.
Don't use the [entry, entry]
array syntax for the entry property
. You basically bail out the whole build because the []
represents in the webpack codebase what we call MultiModule
and is treated as a flexible CommonJS style "concatenation". If you need to load polyfills before your entrypoint, just import
it at the top of your entry file.
webpack does this by default in mode: "production"
so I'm not sure why this is being set here (unless you were copy-pasta'ing from other configs [which is heavily discouraged by the webpack team])
Only under extreme scenario's do you want to be messing with context
. If you change context, you are chaning how webpack also resolved the default location of node_modules
. Removing this allows you to succesfully add import "@babel/polyfill"
in your entry file and also let's you drop the entry
property (since webpack already defaults to ./src/
and you have .tsx
as autoresolvable).
So once you fix all those things (which I hope you do!), then you can use a tool like webpack-bundle-analyzer to see that UI fabric (which is exactly the format it is supposed to be in), only takes up about ~40kb (uncompressed):
I hope that all of this information leads you to understand that you should be aware of all the things tools are doing and why. If some of this information is new and surprising, then I would love for you to submit a new issue, or update our webpack documentation.
I will say that we've been digging into bundle size here and have found a few instances where we still had sideEffects: true instances, which would fight against tree shaking. We have a PR open to address some of this, which will help a number of scenarios.
This does assume however that you do what @TheLarkInn mentioned above to make sure you're consuming es6 modules.
@TheLarkInn Thanks for the quick and thorough response, it's much appreciated. As I said before I expected it was something I was doing wrong so thanks for setting a newbie like me straight with copy-pasta'ing and reminding me it's important to read the documentation of ALL the tools I use.
After following the instructions and reading up on some documentation I found that my output from webpack-bundle-analyzer was the same as yours... But that in itself seemed like a problem to me. It appears that a lot of the uifabric components were "bailed out" and as a result bloat up the index.tsx and src instead as shown here:
These instructions did cut off around a 100kb but as the terminal shows bundlejs still ends up at 400+kb which I gather is non-ideal in webpack's worldview. And I respect webpack warning me about this because while at the moment I am still in the throws of the learning stage I would like to get to a point where I am following best practice and creating a killer app. @dzearing is what I'm observing in line with what you referred to as further optimizations regarding sideEffects: true in a open PR? I have a new commit up so feel free to check if there's something else I'm missing here. And again, Thanks a lot guys!
There are definitely some bailouts that can be solved. (For example I would encourage you to submit bugs for the injected variables).
But only about 40-50kb of that bundle is from fabric (and the rest is other dependencies last I checked (on mobile now lol)).
Just clone the repo and bundle it urself until v7 comes out, or just copy and paste the .min file into your public folder and use unpkg / the cdn , ive reorganized this repo 3 times locally ..its not worth messing with any more until the next semver
That’s something to consider Jeremy for if/when I reach that need. Realistically, I’m a lot of time out from deploying something where it matters whether something is 255kb instead of 400ish kb. I come from the world of Dynamics 365 and angularjs customizations so this retrieval amount is quite small related to what I’m used to. So for now I’ll hang tight and wait for the new version of the package since it seems they are already tracking on improvements to the size. Thanks everyone!
FWIW @jeremy-coleman we've just merged one change which should bring bundle sizes down. You can see results in the PR we just merged: https://github.com/OfficeDev/office-ui-fabric-react/pull/8032
Note that really webpack 4 is what we're focused on.
We have additional improvments coming as well to reduce the graph edges which pull in unneeded code into the bundle.
I can fix this in a day or two, will you use it if i fix it? so tired of dealing with countless issues related to packaging.