Tree shaking in Flagpack

[zoeyfrisart]

This discussion aims to provide a place for feedback in regards to the planned implementation of Tree shaking in Flagpack.

The problem

Currently Flagpack doesn't support Tree shaking, this is because we load in all the flags to be able to dynamically decided which flag the user needs. This however poses a problem, as in theory the user could need all flags. As such static analysis tools are unable to tree shake the core.

Proposed solution

After carefull consideration, we have come to the following plan of action to implement treeshaking.

1. The core will be rewritten to use Typescript, so that we can provide type checking on the flags.
2. We will remove the imageUrl and isoToCountryCode from the default exports. We might include isoToCountryCode as a named import, however note that this would still load all the countries (around 16kb of data)
3. We will change the build process to purely generate all the svg files (with the different implementations (ALPHA2, ALPHA3, numeric) in the different sizes.
4. We need to not import countryList.json in the main code, however we should still include that file as a named import
5. We will mark the flags as a directory that should be included in the package.
6. The different packages will change their implementation to do dynamically require the SVG file, instead of using the imageUrl function.

So for example the react-flagpack would parse flags as following instead:

<img src={require(`flagpack-core/flags/${size}/${name}.svg`)} />

Possible issues with this implementation

The following possible issues have been identified, we aim to mitigate these as much as possible.

1. It will dynamically require the svg's, this might cause issues with the build tools of a user.
2. We need to add documentation for build tools if changes are needed
3. We should add checks to the packges for length of input (max 6)
4. Inform that the input shouldn't be user input. As it's dynamic require to the server. (possible security issue, needs further investigation)

This is an ongoing process, and is subject to change, and open to feedback.

[zoeyfrisart]

This implementation has been chosen instead of the earlier attempted methods here:
#31 #30
Yummygum/react-flagpack#29

[daoneandonly]

Current issues with our approach

With Flagpack 2.0 released, we are currently saving the flag SVGs using flagpack-core. Within our framework packages, we are using dynamic imports to import these svg images from this seperate package and serve them to the project in which this framework package is used.

While this worked in testing with a CRA and CNA in a local package, this turned out be a bit more problematic in a live environment. Dynamic imports are relatively new, and don't seem to work well together with requiring SVG's. This seems to be the downfall of both our current react-flagpack implementation and proposed svelte-flagpack approach, which can be found here. To reiterate: dynamic imports could work and importing SVG's could work, but using these together to import SVG's from a seperate package seems to be what breaks our packages.

A different approach

We have quite a few CLI scripts in our flagpack-core package, and I think this might be the approach we should take with our framework packages as well. My proposed approach is as follows:

• Use flagpack-core to store the flags as we do now
• In our framework package, use a node script to generate our components in the corresponding framework, based on flagpack-core (this can be something that happens at install, or something that is set in its current version).
• (dynamically) import these components, into our main Flag component within the framework package.
• Export the Flag component to be used in a project.

With this approach we solve a few things.

• Trying to make one-size-fits-all solution and instead just use flagpack-core as a image source, like our current approach.
• Making our framework package work out of the gate, without requiring any additional tooling.
• No longer dynamically importing SVG's from a separate package

Meanwhile

In the meantime we should probably revert flagpack-react as seems to be broken currently as seen here