Skip to content
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.

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

react-redux-firebase dist output could be reduced further #573

Closed
AdrienLemaire opened this issue Nov 26, 2018 · 6 comments
Closed

react-redux-firebase dist output could be reduced further #573

AdrienLemaire opened this issue Nov 26, 2018 · 6 comments

Comments

@AdrienLemaire
Copy link

AdrienLemaire commented Nov 26, 2018

Do you want to request a feature or report a bug?
I suppose it's both a bug and a feature

What is the current behavior?
Lot of unused code is built in the dist output.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via codesandbox or similar.

grep -R "firebase" src | grep import
src/sagas/user.js:import {actionTypes as firebaseActionTypes} from "react-redux-firebase";
src/sagas/snackbar.js:import {actionTypes as fbActionTypes} from "react-redux-firebase";
src/components/dialogs/EmailSignupForm.js:import {withFirebase} from "react-redux-firebase";
src/components/dialogs/SignUpForm.js:import {withFirebase} from "react-redux-firebase";
src/components/Header/AnonymousMenu.js:import {actionTypes as fbActionTypes, withFirebase} from "react-redux-firebase";
src/components/Header/LoggedInMenu.js:import {withFirebase} from "react-redux-firebase";
src/configureStore.js:import {reactReduxFirebase} from "react-redux-firebase";
src/configureStore.js:import firebase from "firebase/app";
src/configureStore.js:import "firebase/auth";
src/configureStore.js:import {firebaseConfig, rrfConfig} from "./constants";
src/reducers/index.js:import {firebaseReducer} from "react-redux-firebase";
src/reducers/ui.js:import {actionTypes as fbActionTypes} from "react-redux-firebase";

Resulting in

What is the expected behavior?
The dist output should only contain code that I'm using in my application. I was surprised to notice stuff like firestoreConnect.js and withFirestore.js, when I'm never using them.
The whole react-redux-firebase/es thing is 13.81kb gzipped, and it would be much smaller if webpack could remove the unused code.

Note: I tried to do specific imports to help webpack, eg

- import {actionTypes} from "react-redux-firebase";
+ import {actionTypes} from "react-redux-firebase/constants";

But I'm getting eslint errors import/no-unresolved: Unable to resolve path to module 'react-redux-firebase/constants'
It seems to work with the following

import {actionTypes} from "react-redux-firebase/es/constants";

But is that really the proper way to go ?

@AdrienLemaire
Copy link
Author

I tried to replace all my imports to test the dist output, and ... it grew 😱

grep -R "firebase" src | grep import | grep firebase
src/sagas/user.js:import {actionTypes as firebaseActionTypes} from "react-redux-firebase/es/constants";                                                                                    
src/sagas/snackbar.js:import {actionTypes as fbActionTypes} from "react-redux-firebase/es/constants";                                                                                      
src/components/dialogs/EmailSignupForm.js:import withFirebase from "react-redux-firebase/es/withFirebase";                                                                                 
src/components/dialogs/SignUpForm.js:import withFirebase from "react-redux-firebase/es/withFirebase";                                                                                      
src/components/Header/AnonymousMenu.js:import {actionTypes as fbActionTypes} from "react-redux-firebase/es/constants";                                                                     
src/components/Header/AnonymousMenu.js:import withFirebase from "react-redux-firebase/es/withFirebase";                                                                                    
src/components/Header/LoggedInMenu.js:import withFirebase from "react-redux-firebase/es/withFirebase";                                                                                     
src/configureStore.js:import reduxFirebase from "react-redux-firebase/es/enhancer";
src/configureStore.js:import firebase from "firebase/app";
src/configureStore.js:import "firebase/auth";
src/configureStore.js:import {firebaseConfig, rrfConfig} from "./constants";
src/reducers/index.js:import {firebaseReducer} from "react-redux-firebase/es/reducer";
src/reducers/ui.js:import {actionTypes as fbActionTypes} from "react-redux-firebase/es/constants";

It grew from 13.81Kb to 14.36kb, and as we can see, the unwanted firestoreConnect & co are still there :(

@prescottprue
Copy link
Owner

@Fandekasp What version of webpack are you using? Interested to replicate this. The es or commonjs version should be used by default and treewalking should hopefully only pull in what you are using (as you mentioned).

@AdrienLemaire
Copy link
Author

@prescottprue sorry about that, forgot to specify the versions

  • webpack 4.26.1
  • terser-webpack-plugin 1.1.0
  • react 16.6.3
  • redux 4.0.1
  • react-redux 5.1.1
  • react-redux-firebase 2.2.3
  • @babel/core 7.1.6
  • babel-loader 8.0.4
  • @babel/plugin-syntax-dynamic-import 7.0.0
  • eslint-import-resolver-babel-plugin-root-import 1.1.1
  • eslint-import-resolver-typescript 1.1.1
  • eslint-plugin-import 2.14.0

Our setup is pretty complex, so a large number of things could be the culprit for breaking the build assuming react-redux-firebase has no issues. It would take me some time to build a simple project to replicate it.
Are you not noticing that all files are always imported even when not used in your own projects ?

small bits from the webpack conf
{
  module: {
    rules: [
	  {
	    test: /\.(js|ts|tsx)$/,
	    exclude: /node_modules/,
	    use: {
	      loader: "babel-loader",
	      options: {
	        babelrc: true,
	        cacheDirectory: true,
	      },
	    },
	  },
	  {
	    test: /\.js$/,
	    exclude: /node_modules/,
	    loader: WebpackStripLoader.loader("console.log"),
	  }
    ]
  },
  optimization: {
    minimizer: isProduction
      ? [
          new TerserPlugin({
            cache: true,
            extractComments: true,
            parallel: true,
            terserOptions: {
              toplevel: true,
              compress: {
                passes: 3,
                pure_getters: true,
                unsafe: false,
              },
            },
          }),
        ]
      : [],
    runtimeChunk: isProduction,
    splitChunks: isProduction ? {chunks: "all", maxSize: 1200000} : false,
  },
  output: {
    path: path.resolve(__dirname, bundlePath),
    publicPath,
    filename: isProduction ? "[name].[chunkhash].js" : "[name].js",
    chunkFilename: isProduction ? "[name].[chunkhash].js" : "[name].js",
  },
  performance: isProduction
    ? {
        maxEntrypointSize: 250000,
        maxAssetSize: 1000000,
      }
    : {hints: false}
}
eslintrc
extends:
  - eslint-config-with-prettier
  - plugin:redux-saga/recommended
  - plugin:security/recommended

settings:
  react:
    version: 16.6.0
  import/resolver:
    babel-plugin-root-import: {}
    typescript: {} # use tsconfig.json
  import/parsers:
    typescript-eslint-parser: [ .ts, .tsx ]


parser: babel-eslint

plugins:
  - immutable
  - import
  - material-ui
  - redux-saga
  - security
  - typescript

parserOptions:
  ecmaVersion: 2018
  sourceType: "module"
  allowImportExportEverywhere: false
  codeFrame: false
  ecmaFeatures:
    legacyDecorators: true
    jsx: true

globals:
  firebase: true
  requirejs: true
  window: true
  _: true
  expect: true
  jest: true
  # enzyme vars (check jestsetup.js)
  shallow: true
  render: true
  mount: true

env:
  amd: true # for require.js (request() and define())
  browser: true
  commonjs: true
  es6: true
  mocha: true

rules:
  class-methods-use-this: 1
  flowtype/no-types-missing-file-annotation: 0
  immutable/no-mutation: 2
  import/named: 1
  import/order: error
  import/no-extraneous-dependencies: 0
  import/no-unresolved: error
  jsx-a11y/anchor-is-valid:
  - error
  - components:
    - Link
    specialLink:
    - to
  no-restricted-syntax:
    - error
    - selector: 'ForInStatement'
      message: 'for..in loops iterate over the entire prototype chain, which is virtually never what you want. Use Object.{keys,values,entries}, and iterate over the resulting array.'
    - selector: 'LabeledStatement'
      message: 'Labels are a form of GOTO; using them makes code confusing and hard to maintain and understand.'
    - selector: 'WithStatement'
      message: '`with` is disallowed in strict mode because it makes code impossible to predict and optimize.'
  no-else-return:
    - "error"
    - allowElseIf: true
  one-var:
    - error
    - var: "always"
  prefer-destructuring:
    - error
    - object: false
  react/jsx-filename-extension: 0
  react/forbid-foreign-prop-types: 1
  # We'll delete proptypes after finishing typescript integration
  react/prop-types: [2, {skipUndeclared: 1}]
  react/sort-prop-types: "error"
  sort-imports: 0
  sort-keys: 0
  sort-vars: 2
  strict: 0
  typescript/class-name-casing: "error"
  typescript/no-unused-vars: "error"
  typescript/type-annotation-spacing: "error"

overrides:
  - files: ["*.ts", "*.tsx"]
    parser: "typescript-eslint-parser"
    parserOptions:
        jsx: true

# vi: ft=yaml

prescottprue pushed a commit that referenced this issue Dec 15, 2018
* fix(tests): use node 6 as target during tests
* feat(core): remove getFirebase
* feat(core): add sourcemaps (now that lib/es code is minified)
@prescottprue prescottprue mentioned this issue Dec 15, 2018
3 tasks
prescottprue added a commit that referenced this issue Dec 15, 2018
* feat(HOCs): switch `firestoreConnect` and `firebaseConnect` to use `componentDidMount` over `componentWillMount` - #564 
* feat(core): New react context API working with HOCs (`firebaseConnect`, `firestoreConnect` `withFirebase`, and `withFirestore`) - #581
* feat(core): support `react-redux` v6 by removing usage of `context.store` (used to be added by `Provider` from `react-redux`)
* feat(core): Added `ReactReduxFirebaseContext` and `ReduxFirestoreContext` (with associated providers `ReactReduxFirebaseProvider` `ReduxFirestoreProvider` respectively)
* fix(core): shrink build sizes considerably by adding `babel-preset-minify` - #573
* feat(deps): update `hoist-non-react-statics` to `^3.1.0`
* feat(deps): upgrade to `babel^7`
* feat(deps): Update to `webpack^4` (along with `webpack-cli`)
* feat(deps): update react peer dependency to `^16.4` for new Context API (matches `react-redux`)
@prescottprue
Copy link
Owner

Released I released v3.0.0-alpha to the next tag (it can be installed with npm i --save react-redux-firebase@next). Also published the v3.0.0 section of the docs which explains how to install and includes a migration guide.

@Fandekasp do you want to give that a try - it has the babel minify plugin and it seems to have shrunk all of the builds considerably. It also is using the newest webpack, so it seems like tree shaking may work better (or as expected at least).

If build sizes are still not acceptable we can reopen, but from the looks of it things are much better.

@AdrienLemaire
Copy link
Author

@prescottprue I didn't expect v3 to come out so fast. Impressive 👏

@prescottprue
Copy link
Owner

@Fandekasp Thanks 😄 . It may still change up a bit, but wanted to get some out since so many in the community were interested.

@prescottprue prescottprue mentioned this issue Oct 9, 2019
10 tasks
prescottprue added a commit that referenced this issue Oct 12, 2019
All changes from v3.0.0 pre-release versions including:
* [X] Hooks (`useFirebase`, `useFirebaseConnect`, `useFirestore`, `useFirestoreConnect`)
* [X] Rebuild on stable React Context API
* [X] Support react-redux v6 - [#581](#581)
* [X] Support for react strict mode - [#564](#564)
* [X] Improved bundle size support (should include way to audit size) - [#573](#573)
* [X] Support/Docs for stable react context API
* [X] Switch `firebaseConnect` and `firestoreConnect` to using `componentDidMount` in place of `componentWillMount` which is deprecated in newer react versions (as [described here](https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html#fetching-external-data))


More details available in [the v3.0.0 roadmap](https://github.com/prescottprue/react-redux-firebase/wiki/v3.0.0-Roadmap)
mirdavion pushed a commit to mirdavion/react-redux-firebase that referenced this issue Mar 18, 2021
All changes from v3.0.0 pre-release versions including:
* [X] Hooks (`useFirebase`, `useFirebaseConnect`, `useFirestore`, `useFirestoreConnect`)
* [X] Rebuild on stable React Context API
* [X] Support react-redux v6 - [#581](prescottprue/react-redux-firebase#581)
* [X] Support for react strict mode - [#564](prescottprue/react-redux-firebase#564)
* [X] Improved bundle size support (should include way to audit size) - [#573](prescottprue/react-redux-firebase#573)
* [X] Support/Docs for stable react context API
* [X] Switch `firebaseConnect` and `firestoreConnect` to using `componentDidMount` in place of `componentWillMount` which is deprecated in newer react versions (as [described here](https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html#fetching-external-data))


More details available in [the v3.0.0 roadmap](https://github.com/prescottprue/react-redux-firebase/wiki/v3.0.0-Roadmap)
prodev90 added a commit to prodev90/react-redux-firebase-sample that referenced this issue Jan 4, 2023
All changes from v3.0.0 pre-release versions including:
* [X] Hooks (`useFirebase`, `useFirebaseConnect`, `useFirestore`, `useFirestoreConnect`)
* [X] Rebuild on stable React Context API
* [X] Support react-redux v6 - [#581](prescottprue/react-redux-firebase#581)
* [X] Support for react strict mode - [#564](prescottprue/react-redux-firebase#564)
* [X] Improved bundle size support (should include way to audit size) - [#573](prescottprue/react-redux-firebase#573)
* [X] Support/Docs for stable react context API
* [X] Switch `firebaseConnect` and `firestoreConnect` to using `componentDidMount` in place of `componentWillMount` which is deprecated in newer react versions (as [described here](https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html#fetching-external-data))


More details available in [the v3.0.0 roadmap](https://github.com/prescottprue/react-redux-firebase/wiki/v3.0.0-Roadmap)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants