Wrapping and reexporting dependencies

[ismay]

Wrapping and reexporting dependencies is a pattern that has come up a couple times in our apps/libs and discussions. Forgive me for bringing this up again, but after thinking about it I still wouldn't be able to explain to someone else why exactly we do this. So I thought it'd be good to consolidate our reasoning behind why we're doing this and have it written down for future reference.

To clarify what I'm talking about, for example; we wrap and reexport final-form with our ui-forms library, we do the same with the prop-types package for our @dhis2/prop-types package and I believe we do something similar for our cli tooling package.

My understanding is that we do this for these reasons:

1. We have quite a lot of apps to maintain. Wrapping these dependencies cuts down on manually having to upgrade the reexported packages for each app
2. We want to make sure that we're only using a single version of a package
3. It allows us to catch any bugs at the wrapping package, instead of at the app

Please correct me if the above is wrong, or if I've missed anything!

[varl]

Instead of trying to carve out the time to sit down and document all the reasons at once, I will incrementally add them.

[varl]

First of all, let's separate the terms re-exporting and wrapping. If we agree that re-export is a technique where we simply re-export the library from a different namespace (ours) and wrapping is a technique where we wrap the library and may choose to re-export part of it (or not), then it is easier to rationalise about when the techniques can be applied and why.

Therefor I propose that we use the definitions:

Re-exporting

// re-export.js
import React from 'react'

export { React }

Wrapping

// wrapping.js
import React from 'react'

const Component = React.Component

export {
  Component
}

[varl]

Manage the attack surface

Wrapping (regardless of re-exporting) only the parts of a library that is actually used is a technique that is used to manage the attack surface.

This means that you will only import React once in the application in the wrapper module, and then only expose the parts from React that you need.

1. It makes it easier to audit what parts of the library is in use.
2. It restricts the attack surface.
3. It enables additional mitigations to be implemented in the wrapper layer as countermeasures to known vulnerabilities.

This is generally not common in front-end applications (yet?). I'll leave any speculation aside about why that. It is mentioned in different parts of the OWASP documentation, and while it may be annoying for developers (one additional hoop 🦁🗯), it is a Good™️ practice to consider.

[varl]

This is related to the discussion here: https://github.com/dhis2/ui/pull/35#issuecomment-618223542

One practical argument is that if we export * from our re-exported libraries, then any breaking change they do upstream forces us to release a breaking change for our library.

Example:

Someone does import { FinalFormFeature } from '@dhis2/ui-forms, and then upstream drops FinalFormFeature from their package and publishes a breaking change.

In @dhis2/ui-forms we update our code to not rely on FinalFormFeature, but figure it is not a breaking change for us, so we publish @dhis2/ui-forms with a fix(deps): update final-form to new major.

Somewhere, elsewhere, a developer has imported FinalFormFeature from our @dhis2/ui-forms package, and bumps the patch version of our library and poof. Now his code has broken without realizing why. All he did was bump the patch version and we silently shipped a breaking change to him.

Problem:

Yes, it's easier for us to just re-export * and it is more future-proof in the sense that we can transparently update our dependencies and have those changes reflected through our public API.

The point of being explicit in re-exporting is that we get a safety mechanism in that our build will fail if we attempt to re-export an export that no longer exists in the library we are re-exporting. Our build would fail before our users' builds would fail. I think that this is an important responsibility that we have.

Alternative:

We can gather up all the named exports for FinalForm and perhaps ReactFinalForm in a namespace that we can provide jsdocs for explaining the risks:

/**
 * @module
 * @desc Allows direct access to the FinalForm library. Please note that this is considered advanced
 * usage and that you need to stay up to date with breaking changes in the FinalForm library.
 * @return FinalForm 
 */
export { * as FinalForm } from 'final-form'

/**
 * @module
 * @desc Allows direct access to the ReactFinalForm library. Please note that this is considered
 * advanced usage and that you need to stay up to date with breaking changes in the FinalForm
 * library.
 * @return ReactFinalForm 
 */
export { * as ReactFinalForm } from 'react-final-form'

In addition to that, we can provide "safe" named re-exports for react-/final-form features that are absolutely essential as we did before if we want to.

[varl]

In dhis2/ui#35 we settled on and recommend for situations where we want to wrap a vendor library:

• https://github.com/dhis2/ui/blob/alpha/packages/forms/src/index.js#L14-L32
• https://github.com/dhis2/ui/blob/alpha/packages/forms/src/__tests__/index.test.js

In dhis2/prop-types we also ensure that we do not overwrite any exports:

• https://github.com/dhis2/prop-types/blob/master/src/__tests__/index.test.js#L40-L44

Which we will also want to do for our internal packages in dhis2/ui as well.

We think this strikes a good balance of:

• control over our dependencies and their versions
• flexibility for our consumers
• documentation about the risks and expectations
• automatic safety nets when upgrading dependencies to catch API changes

[varl]

https://github.com/dhis2/notes/issues/67#issuecomment-618856396 can be considered the proposal for how to handle re-exported and wrapped libraries.

Ready for discussion in a Frontend Meeting.

[varl]

https://github.com/dhis2/prop-types/pull/133

[amcgee]

This is unfortunately late since I wasn't aware of this discussion when it happened, but I think this approach could be problematic from a tree-shaking and bundle size perspective. Doing this (as we do in ui-forms):

import * as ReactFinalForm from 'react-final-form'
import * as FinalForm from 'final-form'

export { FinalForm }
export { ReactFinalForm }

Not only requires a kind of ugly import syntax:

import { ReactFinalForm, FinalForm } from '@dhis2/ui'

const { useField } = ReactFinalForm
const { FORM_ERROR } = FinalForm

I think it also prevents anything from final-form or react-final-form from being tree-shaken. This should be tested and confirmed, my intuition may be wrong, but in cases where we might re-export a library with a large surface-area this could negatively affect application bundle sizes.

[amcgee]

Relevant to this discussion : https://classic.yarnpkg.com/blog/2018/04/18/dependencies-done-right/

[varl]

If need be it can be changed; it was never formally decided on in the frontend forum, so it is fair game as I see it.

To do so we need a solid alternative proposal that goes further to align our workflow for apps. Maybe we can figure out something with peerDeps without winding up in a situation where each app needs to stay in sync with ui-forms, final-form, and react-final-form in isolation?

Since the re-export strategy is implemented and active, it should be trivial to see if anything can be tree-shaken or if it is permafrost in the bundle. Adding ui-forms to a platform app that generates a source map explorer report should be a quick first check.

[ismay]

@amcgee To illustrate what I meant yesterday with the "larger than" version range. This is what the npm release post for peerDependencies recommends:

One piece of advice: peer dependency requirements, unlike those for regular dependencies, should be lenient.

Because, as @HendrikThePendric mentioned, if the ranges are too strict, users won't be able to satisfy the version requirements with a single version, and we're back where we started.

To that end, my strategy in the past has been to specify a larger than range that is as lenient as possible. So say ">3", or something of that nature. Of course, you'll need to be sure that the range is actually valid (so that none of the versions that this requirement resolves to will break your library). You can decrease/increase the range as needed for safety of course, but this just to illustrate what I meant.