feat(treeshaking): A start on making the component tree shakable

zoeyfrisart commented 3 years ago

This attempts to create a base for tree shaking, this change is related to changes to the core. Currently however this implementation isn't working as intended yet.

Suggested change: We create 2 different components

Flag (The default usage, which allows for the current use)
PerfFlag (Name should probably be different) (Doesn't accept Code and instead only accepts a Flag object from the core)

Backstory

The react component currently uses the imageUrl method, this has the issue that this prevents tree shaking as the core has no way to know which elements are used and which are unused.

My suggestion

Would be to allow users to import a flag from the core and pass that to the component, this way the user can opt-out of loading all flags.

The most desired here would be that a user can do: import { PerfFlag } from 'react-flagpack', this works now however it doesn't tree shake (because we have side effects caused by scss). So the user needs to now do 'react-flagpack/dist/PerfFlag.esm'

Problems that still happen:

This partially works, the react component is smaller and doesn't have all the flags. However the core seems to import all the SVG's and flags even if we only export a small amount of them.

Possible fixes for that would be:

Change core even more so that each flag generates a JS file, so the user can import NL from 'flagpack-core/NL' (this however wouldn't be most optimal as that becomes a lot when using a lot of flags)
Find a method that allows us to have the exports but still tree shake (I know that this is possible but I can't remember the way)

Test environment

I used the following environment to test the changes in (Note that this PR depends on the PR in the core)

PerfFLag:

import Head from 'next/head'
import { PerfFlag } from 'react-flagpack/dist/PerfFlag.esm'
import { AD, AI, AL, CC } from 'flagpack-core/dist/allFlags.esm'

import styles from '../styles/Home.module.css'

export default function Home() {
  return (
    <div className={styles.container}>
      <Head>
        <title>Create Next App</title>
        <link rel="icon" href="/favicon.ico" />
      </Head>

      <PerfFlag flag={AD}/>
      <PerfFlag flag={AI} />
      <PerfFlag flag={AL} />
      <PerfFlag flag={CC} />
    </div>
  )
}

Flag:

import Head from 'next/head'
import styles from '../styles/Home.module.css'

import Flag from 'react-flagpack'

export default function Home() {
  return (
    <div className={styles.container}>
      <Head>
        <title>Create Next App</title>
        <link rel="icon" href="/favicon.ico" />
      </Head>

      <Flag code="NL" size="l" />
    </div>
  )
}

Next config:

const withImages = require('next-images');
const { BundleAnalyzerPlugin } = require('webpack-bundle-analyzer')

module.exports = withImages({
  webpack: (config, { dev, isServer }) => {
    config.plugins.push(new BundleAnalyzerPlugin({
      analyzerMode: 'static',
      openAnalyzer: false,
      reportFilename: `../../analyzer/report-${Math.random()}.html`
    }))

    return config
  }
});

adamduncan commented 3 years ago

Would love to see where this effort leads.

The library is great, but the inability to import individual flags w/o bringing the whole 18kb package is a showstopper for us.

Would having the individual flags exported as their own named modules be a possibility?

import { FlagNL } from 'react-flagpack';
// or 
import FlagNL from 'react-flagpack/nl';

daoneandonly commented 3 years ago

Hi @adamduncan 👋 We've been prioritising some other things within flagpack, but I agree that some form of this tree shaking should implementated. Anyway, I really like both of these implementations and I think this is definitely something we could work towards.

zoeyfrisart commented 3 years ago

We have internally discussed tree shaking and have come up with an implementation. Feedback is welcome, and will be considered.

https://github.com/Yummygum/flagpack-core/discussions/46

We will close this PR as the implementation here is very different from the approach we aim to implement now.

Yummygum / react-flagpack