Jayden's optimal JavaScript module design topic

[benjie]

Hi folks, sorry I had to drop unannounced yesterday.

Having had to deal with the issue that @jaydenseric raises on many occasions, I fully sympathise with the motivation. Lodash, MUI, and similar frameworks have the pattern of import thing from 'module/thing' and have codemods that can update your codebase to use them, but if you have a single dependency somewhere that does import { thing } from 'module' then all of that effort is pointless. I've had to use patch-package and similar to deeply edit node_modules to change dependencies to importing things from the right place, and it can get old fast!

The very simplest way forward, to try this out, would be to add explicit entrypoints in a flat structure for all our existing exports. The advantage of a flat structure is that a) we don't need to figure out what tree structure we want, b) we don't need to do a semver major release when we refactor internals changing this structure, and c) it's incredibly easy to write a codemod: import { foo as bar } from 'graphql' simply gets rewritten to import bar from 'graphql/foo' and you're done.

I would then suggest that the entire rest of GraphQL.js would be moved into a folder (still in a tree structure) called __private__ or similar, and the 200 or so public entrypoints can import from the __private__/** paths as needed. This would make it very explicit if a user was importing what we consider a "private" utility, and would make it obvious to them that it might break in a patch release.

I echo @enisdenjo's concerns about splitting things up too far without solid use-cases; for example I cannot understand why someone would want GraphQLInt on its own without also wanting e.g. validation or schema or execution; and with any of those things you're going to need GraphQLString and GraphQLBoolean (even if just for the introspection schema). The same goes for the builtin directives. A dogmatic approach to this yields the benefit of simplicity and ease of adoption via codemods, but there is the risk that we're not reaping the full rewards if we introduce more overhead by splitting things up too much - e.g. each module may require fetching (which has HTTP overhead, even when fetched in parallel and multiplexed over HTTP/2+) or may be wrapped in its own little closure (I think this is how webpack works still?) which introduces both size and memory usage overheads. That said, I'm happy to assume that the benefits are worth the cost and I'm in favour of the simplistic dogmatic approach that's easy to codemod.

@jaydenseric perhaps you can lay out some concrete use cases where an application might import just one of the builtin scalars and not use any other part of graphql-js? That would make this approach more clearly the right direction.

On the "no index file" front, I think we should reserve that for the next server major. In the release we introduce the subpaths, I'd re-export everything from the root barrel file but mark it all as deprecated (e.g. via tsdoc) to encourage users to move to the new way. Sometimes it takes a while to fully update a codebase to use the new patterns, so giving users a period when both the root and the subpaths work is going to give them a window to adopt this practice gracefully before we force it upon them in the next semver major. It also means that we can give users an entire release cycle (e.g. v18 -> v19) to prepare for the breaking change coming down the line.

[jaydenseric]

I've touched on some of the points raised in graphql/graphql-js#4442 .

@jaydenseric perhaps you can lay out some concrete use cases where an application might import just one of the builtin scalars and not use any other part of graphql-js? That would make this approach more clearly the right direction.

A great thing about following optimal module design as a rule is you don't have to think very hard about how people might consume your package, and who you might harm by consolidating parts of your API arbitrarily into modules. It's impossible to predict the infinite ways people might want to import or use the API, and if you assume consolidating a certain set of things into one module is ok for most people, it might prohibit someone from using your API in a cool new way you didn't anticipate. Instead of a cool new innovation existing, it just never exists and we all continue on blissfully unaware we missed out.

Here are some examples.

Let's imagine graphql has optimal module design. Here, in graphql-upload I could deep import only GraphQLError and GraphQLScalarType:

https://github.com/jaydenseric/graphql-upload/blob/421707f3b4e2b0c18ed9beec8eeaf3a0c942841d/GraphQLUpload.mjs#L3

Then, later in the sibling test module, I could deep import only parseValue:

https://github.com/jaydenseric/graphql-upload/blob/421707f3b4e2b0c18ed9beec8eeaf3a0c942841d/GraphQLUpload.test.mjs#L6

Then, while working on GraphQLUpload:

• When opening the file in VS Code, TS can load the editor tab faster because it's not parsing and understanding typings for every module in GraphQL.js.
• I could run its unit tests in isolation, and the Node.js runtime would only parse a few hundred lines of code instead of the entirety of the GraphQL.js library. My code coverage would only be collected for the relevant code being tested, and even tho the coverage reporter has code to ignore modules from node_modules, it's more performant to simply never collect all the coverage data for all of the GraphQL.js code that's not "hot" than to later ignore it.

You could argue that, the extra overhead of the runtime and test utilities processing a sub-optimal graphql package and imports is not noticeable to the dev running the tests. But, it's death by 1000 cuts. Think of millions of computers around the world every day processing billions of lines of redundant code. How many tones of greenhouse gas emissions are emitted? If we say performance doesn't matter here, then we are just contributing to the tragedy of the commons where the JS ecosystem feels "slow". One or two dependencies being sub-optimal might not always be perceivable, but when 10,000 in node_modules are sub-optimal we end up wading through quicksand.

Here is a more inspiring example, directly responding to your query about built in scalars.

People have said that importing and using execute pulls in every standard scalar such as GraphQLFloat anyway, so why not keep them in the same module? Well, the current execute implementation might, but what if your company sometimes disables introspection and for those production services without introspection, some schemas happen to not have GraphQLFloat anywhere in the schema. So it's redundant that the code for GraphQLFloat is being loaded at runtime. It's not used! What if you made a composable version of execute, where you can optionally create and pass in resolvers for the introspection ability, that would naturally deep import GraphQLFloat etc. in it's implementation where it's used, and if you don't compose introspection in, you never load and parse that code. Imagine that more performant version of execute works well, so you want to contribute the better design upstream to GraphQL.js. Now the whole world benefits from more composable and efficient code. In an unfortunate present reality where all the standard scalars are in one file, your runtime will pull them all in regardless if you have introspection enabled, and importing just one standard scalar would pull every one in, so there goes the motivation to create and contribute a composable execute API.

[dburles]

Hey @benjie I'm one of @jaydenseric's colleagues that he mentioned in the meeting. It's great to see this approach being given a strong consideration and I can very much sympathise with the many frustrations relating to what it solves.

I echo @enisdenjo's concerns about splitting things up too far without solid use-cases; for example I cannot understand why someone would want GraphQLInt on its own without also wanting e.g. validation or schema or execution; and with any of those things you're going to need GraphQLString and GraphQLBoolean (even if just for the introspection schema). The same goes for the builtin directives.

As Jayden mentioned. One export per file is an advantage here again by negating the need for contributors to make assumptions around whether certain things should be grouped together into one module. In addition, if part of that decision making process requires attempting to predict use cases, that may be a fruitless exercise.

A dogmatic approach to this yields the benefit of simplicity and ease of adoption via codemods, but there is the risk that we're not reaping the full rewards if we introduce more overhead by splitting things up too much - e.g. each module may require fetching (which has HTTP overhead, even when fetched in parallel and multiplexed over HTTP/2+) or may be wrapped in its own little closure (I think this is how webpack works still?) which introduces both size and memory usage overheads. That said, I'm happy to assume that the benefits are worth the cost and I'm in favour of the simplistic dogmatic approach that's easy to codemod.

The benefits definitely outweigh the cost here and it’s unrealistic maintaining multi export modules as a solution to this perceived overhead.

On the "no index file" front, I think we should reserve that for the next server major. In the release we introduce the subpaths, I'd re-export everything from the root barrel file but mark it all as deprecated (e.g. via tsdoc) to encourage users to move to the new way. Sometimes it takes a while to fully update a codebase to use the new patterns, so giving users a period when both the root and the subpaths work is going to give them a window to adopt this practice gracefully before we force it upon them in the next semver major. It also means that we can give users an entire release cycle (e.g. v18 -> v19) to prepare for the breaking change coming down the line.

I like the approach you described here if the plan is to introduce deep imports in a subsequent major. However, I’m not convinced that it’s preferable to introducing deep imports as part of v18. I think v18 introducing both ESM and optimal modules are solid reasons to migrate and also serve to hasten adoption.