jannikbuschke / formik-antd

Simple declarative bindings for Ant Design and Formik.
https://codesandbox.io/s/github/jannikbuschke/formik-antd-example
MIT License
587 stars 81 forks source link

Changing imports to reduce bundle size? #63

Open joshsmith opened 5 years ago

joshsmith commented 5 years ago

I just spent a couple hours tonight trying to figure out why my import on demand per Ant's docs was not working. Turns out that it was a couple imports from formik-antd.

I wonder if it's possible for formik-antd to import similarly, e.g. instead of import { Form } from 'antd' you would do import Form from 'antd/lib/form' or import Form from 'antd/es/form'.

Thanks in advance!

joshsmith commented 5 years ago

Also, Ant strongly recommends keeping imports as you have them and instead using babel-plugin-import.

jannikbuschke commented 5 years ago

Yes this is totally what I would want. Just do normal imports, let babel-plugin-import transform and have the import on demand thing working. Help of any sort here is appreciated as I have only little knowledge about babel and the js import/modul/css tricky details...

jannikbuschke commented 5 years ago

A first step should be to get the import on demand working. Others #24 have mentioned that it is currently not working.

joshsmith commented 5 years ago

Okay, I can try to look into this later, though I'm having issues understanding even how ant's own exports work exactly (due, unfortunately, in large part to much of the conversation in their issues being in Mandarin).

claneo commented 5 years ago

This is my babel-plugin-import config:

[
  'import',
  {
    libraryName: '@jbuschke/formik-antd',
    style: libraryName => {
      let name = libraryName.replace('@jbuschke/formik-antd/es/', '');
      if (name === 'FormikDebug') return false;
      if (name === 'SubmitButton') name = 'Button';
      name = name[0].toLowerCase() + name.substr(1);
      name = name.replace(/([A-Z])/g, $1 => `-${$1.toLowerCase()}`);
      return `antd/es/${name}/style`;
    },
    libraryDirectory: 'es',
    camel2DashComponentName: false,
    transformToDefaultImport: false,
  },
  'formik-antd',
]

So my suggestion is to use same filename style with antd, and include style entry file, which looks like this:

es
├─date-picker
│ ├─index.js
│ └─style
│   ├─index.js
│   └─css.js
├─form
│ └─...
└─...

style/index.js and style/css.js just import antd/lib/xxx/style and antd/lib/xxx/style/css.js

After doing that, we can use a simple babel-plugin-import config:

[
  'import',
  {
    libraryName: '@jbuschke/formik-antd',
    style: true,
    // or
    // style: 'css',
    libraryDirectory: 'es'
  },
  'formik-antd',
]

@jannikbuschke

joshsmith commented 5 years ago

@claneo thanks so much for sharing this!

@jannikbuschke I would definitely second the approach suggested here.

jannikbuschke commented 5 years ago

@joshsmith I am currently doing so. There is version 1.3.0-beta.0 where I tried to apply above to the AutoComplete control. I am now trying to test.

jannikbuschke commented 5 years ago

currently I cannot even get plugin-import to work for antd. My .babelrc looks like this:

{
  "plugins": [
    [
      "import",
      {
        "libraryName": "antd",
        "libraryDirectory": "es",
        "style": "css"
      }
    ] // `style: true` for less
  ]
}

As create-react-app comes with babel included I expected this to work out of the box. But somehow it doesnt.

// following ant designs docs https://ant.design/docs/react/use-with-create-react-app it seems it is necessary to use react-app-rewired and customize-cra.

jannikbuschke commented 5 years ago

@claneo your suggestions seem to work. I released v 1.3.0-beta.6 which exports the AutoComplete component with your suggestions.

As per ant designs documentation I use react-app-rewire and customize-cra (with create-react-app 3) with following config-overrides.js (root level):

const { override, fixBabelImports } = require('customize-cra');

module.exports = function override(config, env) {
    // do stuff with the webpack config...
    return config;
};
module.exports = override(
    fixBabelImports('antd', {
        libraryName: 'antd',
        libraryDirectory: 'es',
        style: 'css',
    }),
    fixBabelImports('formik-antd',
        {
            libraryName: '@jbuschke/formik-antd',
            style: "css",
            // style: true
            // style: 'css',
            libraryDirectory: 'es'
        },
    )
);

The build output is roughly 1 mb. Not sure if this is correct.

claneo commented 5 years ago

@jannikbuschke It's correct. Half of the bundle size comes from antd's icon library, which really annoyed users over the past year.

BTW, it seems like that setting sideEffects field of package.json could help tree shaking out of box, just like what antd do: https://github.com/ant-design/ant-design/blob/master/package.json#L237:L242 related webpack document: https://webpack.js.org/guides/tree-shaking/#mark-the-file-as-side-effect-free

But it only works for js file, styles still need to be imported manually.

It's also the reason of why tree shaking still works while this package import component from antd directly but not antd/es/xxx.

jannikbuschke commented 5 years ago

with source-map-explorer it seems to me that tree-shaking is kind of working. Icons however are included in the build as you expected:

image

didnt yet test the sideEffects option

jannikbuschke commented 5 years ago

It seems the icon bundling issue is recognized and will be fixes as part of ant design 4: https://github.com/ant-design/ant-design/issues/16911

jannikbuschke commented 5 years ago

Tried this https://github.com/Beven91/webpack-ant-icon-loader/issues as a workaround for code-splitting icons but the resulting bundle still shows all icons imported. This is my customize-cra config:

const { override, fixBabelImports } = require('customize-cra');

module.exports = override(
    // add webpack-ant-icon-loader
    (config) => {
        config.module.rules.push({
            loader: 'webpack-ant-icon-loader',
            enforce: 'pre',
            include: [
                require.resolve('@ant-design/icons/lib/dist')
            ]
        });
        return config;
    },
    fixBabelImports('antd', {
        libraryName: 'antd',
        libraryDirectory: 'es',
        style: 'css',
    }),
    fixBabelImports('formik-antd',
        {
            libraryName: '@jbuschke/formik-antd',
            style: "css",
            // style: true
            // style: 'css',
            libraryDirectory: 'es'
        },
    ),
);
joshsmith commented 5 years ago

Tried this https://github.com/Beven91/webpack-ant-icon-loader/issues as a workaround for code-splitting icons but the resulting bundle still shows all icons imported.

Personally, no matter what I have tried here – and I've tried a lot because the total bundle size is huge, and my app is an in-app widget, which just upsets my customers – I could not get this to work. IMO, this is not so much your problem right now as Ant's problem generally. Hopefully v4 drops soon.

edongashi commented 5 years ago

I read this icon workaround somewhere and it worked for me:

config-overrides.js

const path = require('path')
const { override, fixBabelImports } = require('customize-cra')

module.exports = override(
  fixBabelImports('import', {
    libraryName: 'antd',
    libraryDirectory: 'es',
    style: 'css',
  }),
  function (config, env) {
    const alias = (config.resolve.alias || {})
    alias['@ant-design/icons/lib/dist$'] = path.resolve(__dirname, './src/icons.ts')
    config.resolve.alias = alias
    return config
  }
)

icons.ts is a copy paste from antd source but every icon that is not used has been commented out.

jannikbuschke commented 5 years ago

@edongashi can you point directly to this icon.ts file you are mentioning? Im having a hard time finding it.

jannikbuschke commented 5 years ago

I think antd icons are now or will shortly support treeshaking: https://github.com/ant-design/ant-design-icons/pull/96/files

edongashi commented 5 years ago

@jannikbuschke I can't remember and couldn't find it online either. Maybe it's in older commits because as you noted, they are changing the approach.

I uploaded the file here: https://gist.github.com/edongashi/25472166af75197287bd62d14d654363 I would comment all lines and then start uncommenting whichever icons you need.

edongashi commented 5 years ago

Also worth mentioning that I still had issues with bundling while using this library. Treeshaking wasn't working - it was importing a ton of things from antd without ever using it. I tried playing with babel plugins and whatnot but gave up. My (primitive) solution was copying this library (and license) source to a folder inside src and setting "sideEffects" in package.json of CRA:

{
  "sideEffects": [
    "./src/index.{jsx,tsx,js,ts}",
    "./src/i18n.{js,ts}",
    "**/*.css"
  ]
}

This tells the bundler that files which aren't matched for side effects can be safely split.

jannikbuschke commented 5 years ago

I also had problems with tree shaking. There have been also problems with some formik-antd components, however this might be fixed in the newest 1.4 version. Im preparing the documentation here: https://github.com/jannikbuschke/formik-antd/tree/release/1.4#treeshaking--es-imports

Not yet 100% sure if it works, but at least babel imports work (dont need to reference antd.css, just import the components).

edongashi commented 5 years ago

Sounds great. I'll keep an eye for 1.4 and do a cleanup for these things. If I find anything useful I will update. Thanks!

thj-dk commented 4 years ago

I'm having some issues with regards to bundle size and formik-antd version 1.5.1. The formik-antd bundle parse size is rather large:

Here's my customize-cra configuration and component source:

// config-overrides.js
const {
  override,
  addBundleVisualizer,
  addWebpackAlias,
  addLessLoader,
  fixBabelImports
} = require("customize-cra");
const path = require("path");

module.exports = override(
  // add webpack bundle visualizer if BUNDLE_VISUALIZE flag is enabled
  addBundleVisualizer({
    analyzerMode: "static",
    reportFilename: "report.html"
  }),

  fixBabelImports("antd", {
    libraryName: "antd",
    libraryDirectory: "es",
    style: true
  }),
  fixBabelImports("formik-antd", {
    libraryName: "@jbuschke/formik-antd",
    style: true,
    libraryDirectory: "es"
  }),
  addLessLoader({ javascriptEnabled: true }),
  addWebpackAlias({
    "@ant-design/icons/lib/dist$": path.resolve(
      __dirname,
      "src/styles/antd-icons.js"
    )
  })
);
// src/App.tsx
import React from "react";
import {
  Form,
  Input,
  InputNumber,
  Checkbox,
  SubmitButton
} from "@jbuschke/formik-antd";
import { Formik } from "formik";

const App: React.FC = () => {
  return (
    <Formik
      onSubmit={v => console.log(v)}
      initialValues={{ firstName: "", age: 20, newsletter: false }}
      render={() => (
        <Form>
          <Input name="firstName" placeholder="First Name" />
          <InputNumber name="age" min={0} />
          <Checkbox name="newsletter">Newsletter</Checkbox>
          <SubmitButton type="primary">Submit</SubmitButton>
        </Form>
      )}
    />
  );
};

export default App;

What am I missing?

jannikbuschke commented 4 years ago

Unfortunately I'm lacking knowledge in this area. Any help by others is highly appreciated.

thj-dk commented 4 years ago

I'm in the same boat. It would be really great to have this fixed - it's a great library for us using both antd and formik.

edongashi commented 4 years ago

It's weird considering the package has sideEffects=false. Some import somewhere is sticking the modules together.

thj-dk commented 4 years ago

I found another problem with my code sample above. When doing a production build, no antd-related styles are included, only during development using the webpack-dev-server.

Any ideas?

edongashi commented 4 years ago

@thj-dk is your build script in package.json set to "build": "react-app-rewired build"?

Also try setting style: 'css' in fixBabelImports to see if it makes any difference.

JeffJankowski commented 4 years ago

Having the same issue as @thj-dk. I coincidentally just created this issue: https://github.com/jannikbuschke/formik-antd/issues/92

edongashi commented 4 years ago

I found the problem. This library has "sideEffects": false whereas ant design has

"sideEffects": [
  "dist/*",
  "es/**/style/*",
  "lib/**/style/*",
  "*.less"
]

Marking all files as side effect free tells webpack to shake off files that have no used tokens (this happens for styles). I will send a PR now.

thj-dk commented 4 years ago

@edongashi I finally got to test this. sorry for the late reply. Thank you very much for solving this issue.

However, I'm now unable to modify theme variables. Neither of these approaches works.

addLessLoader({
  javascriptEnabled: true,
  modifyVars: {
    hack:
      "true; @import '" +
      path.resolve(__dirname, "./src/styles/theme.less") +
      "'"
  }
})

Or:

addLessLoader({
  javascriptEnabled: true,
  modifyVars: {
    "layout-body-background": "yellow"
  }
}),

Both working using the webpack-dev-server, but not in production builds.

Any ideas?

edongashi commented 4 years ago

Hmm, I'm not sure. This library does not expose any .less files. so I don't know whether adding the ".less" string to sideEffects would have any effect.

Could you try going to node_modules/@jbuschke/formik-antd/package.json and add "*.less" string to sideEffects? Try to rebuild it after that. If it works then we can PR to also include that match.

I'll investigate meanwhile to see if I can find anything.

thj-dk commented 4 years ago

Same result unfortunately.

  "sideEffects": [
    "es/**/style/*",
    "lib/**/style/*",
    "*.less"
  ],

Thank you very much for taking time looking into this.

jannikbuschke commented 4 years ago

In the antd docs it is mentioned that the style prop in babel-import should be true https://ant.design/docs/react/customize-theme#Not-working

thj-dk commented 4 years ago

@jannikbuschke tried different values here, and it doesn't seem to make a difference.

edongashi commented 4 years ago

I tried to reproduce this issue with

modifyVars: {
  "layout-body-background": "yellow"
}

but it works on both dev and prod builds. Do you have an @import statement somewhere in your less files? If yes, please paste it here - maybe there's a problem.

thj-dk commented 4 years ago

Call of the search. Deleting node_modules and reinstalling the packages did the trick. Everything is good now - sorry about that.

jannikbuschke commented 4 years ago

nice. Is there anything that should go into the docs/README?

thj-dk commented 4 years ago

I don't think so. It's pretty straight forward after @edongashi's fix.

HugoLiconV commented 4 years ago

I don't know if I'm doing something wrong but I can't reduce bundle size. here is the code:

const path = require("path");
const { override, fixBabelImports } = require("customize-cra");
const rewireWebpackBundleAnalyzer = require("react-app-rewire-webpack-bundle-analyzer");

module.exports = override((config, env) => {
  fixBabelImports("antd", {
    libraryName: "antd",
    libraryDirectory: "es",
    style: "css"
  });
  return config;
});

and this is my initial bundle size: 492.13 KB build/static/js/2.37dc6e1d.chunk.js 57.68 KB build/static/css/2.93e9ecf4.chunk.css 34.35 KB build/static/js/main.dfd05324.chunk.js 781 B build/static/js/runtime~main.e67f81d3.js 577 B build/static/css/main.9f9a2dd8.chunk.css

and after override the configuration nothing changes

jannikbuschke commented 4 years ago

Hi everyone,

Using this https://github.com/jannikbuschke/cra-antd-x template I get the following build results:

121.26 KB build\static\js\2.9c6a56f0.chunk.js 21.56 KB build\static\css\2.79dd2953.chunk.css 775 B build\static\js\runtime-main.b6d4bee7.js 741 B build\static\js\main.1d9fb7ca.chunk.js 552 B build\static\css\main.1cde33cd.chunk.css

using following config-overrides.js:

const { override, fixBabelImports} = require("customize-cra")
const { addReactRefresh } = require("customize-cra-react-refresh")

module.exports = override(
  fixBabelImports("antd", {
    libraryName: "antd",
    libraryDirectory: "es",
    style: "css",
  }),
  fixBabelImports("formik-antd", {
    libraryName: "formik-antd",
    style: "css",
    libraryDirectory: "es",
  })
)

and rendering one Input, Button and Icon.

For me it looks like tree shaking is working, however I am not sure. Can anyone have a look (check the template or your own projects) and give feedback?