Preact: Preact package should not have a hard dependency on prop-types

Created on 1 Apr 2019  ·  8Comments  ·  Source: preactjs/preact

In https://github.com/developit/preact/commit/069bc8f8a9ea4f514df034d77cadd8160ebb4baa#diff-b9cfc7f2cdf78a7f4b91a753d10865a2R63 a new top-level dependency was added on prop-types@^15.6.2. This package in turn has three dependencies, one of which has an additional dependency of its own. So as a user I end up with six Other People's Codes on my machine (preact, prop-types, loose-envify, js-tokens, object-assign, react-is) even though I'm only using/needing/wanting one (preact).

The prop-types package itself is only needed by a "debug" sub-package hosted in this same repo, but the simple cleanup at #1460 was problematic and had to be reverted. Can a solution be found for the debug folder's usage that still gets us back to a place where preact itself has no dependencies?

Most helpful comment

That's true in theory, but looking at their git history it seems to only receive a couple commits per year. Even the recent ones were mainly maintenance tasks. So I think it would be safe to add our own checkPropTypes.

If anyone wants to take a go at this feel free to do so ✌️

All 8 comments

Hey,

This PR: https://github.com/developit/preact/pull/1401
is taking care of this issue, you can exclude this from your endbundle by just using require sytnax for debug.

if process.env.NODE_ENV!=='production require('preact/debug')

This way it won't be bundled in production.

@JoviDeCroock The work at #1355/#1460 may be somewhat related, but not the core issue here.

Because npm install prop-types (and the trust that entails, recursively!) is not even needed for the primary use case of import 'preact', it shouldn't end up on my machine by default. At best, it slows down npm ci/install/update/audit and at worst… .

While I'm a huge fan of npm modules without any dependencies too, I'm not sure how we could achieve that without alienating our users. From npm's perspective we are just one package: preact. It has no clue about preact/hooks or all the other folders with a package.json in our repo. These sub-packages work thanks to the node resolution algorithm which works independently from npm.

Let's look at the possible scenarios:

  1. We remove prop-types
    1.1 breaks umd and module bundle
  2. We remove prop-types and add options.checkPropTypes where users can supply their own function
    2.1 Another thing users would need to be aware of. May leave a bad impression for users coming from react.
  3. Merge prop-types or a smaller clone into our repo
    3.1 these would fix the issues that plague the other solutions
    3.2 Adds a little bit of maintenance burden, but we only need the checkPropTypes function to be fair

Personally I'd favor solution 3 and I'd be very happy to accept any PR for this :+1:

@marvinhagemeister Thanks, that does make sense. (I hadn't realized exactly what the preact/debug sub-package was either. I understand now it's meant to be used by anyone similar to preact/hooks rather than an internal testing/dev thing…)

When you say "we only need the checkPropTypes function" do you mean just a callable stub (even that does no validation) would suffice?

@natevw yup, preact/debug enables the warnings and the devtools integration for our users :tada:

Replacing it with a stub would kinda defeat the purpose of having any validation at all. The way prop-types works is that the user supplies the actual validator functions and we just need to trigger the actual validation process. This happens via checkPropTypes() which is supplied by prop-types. We do this here:

https://github.com/developit/preact/blob/57c288b46c96ffe136bbbbf561b21dbaa7707bf4/debug/src/debug.js#L72-L75

Looking at the source of that function it doesn't seem to be that big and can probably be golfed down in size.

https://github.com/facebook/prop-types/blob/e32c4900f5ab5fd3acea93e9d2f0d09e4a2f2ceb/checkPropTypes.js#L42

Only potential is if they change something and it breaks our version, but I guess we'd get an issue for that

That's true in theory, but looking at their git history it seems to only receive a couple commits per year. Even the recent ones were mainly maintenance tasks. So I think it would be safe to add our own checkPropTypes.

If anyone wants to take a go at this feel free to do so ✌️

I took a swing at a simplified checkPropTypes over in #1525, feedback appreciated. It's not golfed specifically for code size, but I did leave out as much as I felt we could reasonably get away with.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

SabirAmeen picture SabirAmeen  ·  3Comments

jescalan picture jescalan  ·  3Comments

simonjoom picture simonjoom  ·  3Comments

k15a picture k15a  ·  3Comments

matuscongrady picture matuscongrady  ·  3Comments