Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove collections/mod.ts #2321

Merged
merged 5 commits into from
Jun 9, 2022
Merged

Remove collections/mod.ts #2321

merged 5 commits into from
Jun 9, 2022

Conversation

ry
Copy link
Member

@ry ry commented Jun 8, 2022

Using collections/mod.ts will pull in a whole bunch of unrelated code.
We should not have anti-patterns like this in std.

@ry ry requested review from bartlomieju and kt3k as code owners June 8, 2022 05:04
Using collections/mod.ts will pull in a whole bunch of unrelated code.
We should not have anti-patterns like this in std.
@kt3k
Copy link
Member

kt3k commented Jun 8, 2022

collection utils are relatively independent of each other, and each user should need only small fraction of these. I think removing mod.ts from collection makes sense. But I also want to hear the community's opinions.

@MierenManz
Copy link
Contributor

I'm in favor of removing mod.ts from collection module

@kt3k
Copy link
Member

kt3k commented Jun 8, 2022

Opnions from discoard:

  • mod.ts should stay because mod.ts is easy to remember.
  • mod.ts should stay. Documentation about downside of it in README is enough. (2 people)
  • mod.ts doesn't matter if bundler treeshake unused code.

Copy link
Member

@kt3k kt3k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

collections have more items than when it started. Now it doesn't look a good idea to re-export everything from mod.ts as all unrelated complex algorithms are included there

@ry ry merged commit ae2354d into denoland:main Jun 9, 2022
@ry ry deleted the collections_modts branch June 9, 2022 19:28
@dsherret
Copy link
Member

Whether or not this is an anti-pattern is circumstantial. If I'm just writing a quick script (ex. some data wrangling script) or during development, then using mod.ts is very convenient and provides good auto-complete in the editor... I don't care that it's pulling in a bunch of code that I'm not using because it has no noticable negative impact. When publishing a module to be consumed by others, then yes I would consider this an anti-pattern because it's more ideal to only include what is necessary and not bloat another person's module graph. For an end application, it might matter and it might not.

Essentially, I think we should have left this up to the consumer of std to decide whether to import from mod.ts or not and document the pros & cons. We've removed a nice helper module and made the experience of using std/collections more verbose for scenarios where it just doesn't matter whether code is imported from mod.ts or not.

@jaydenseric
Copy link
Contributor

jaydenseric commented Jun 15, 2022

Index modules that export things from other modules (mod.ts, index.js, etc.) are always an anti pattern. Either they only re-export one thing, in which case the extra index module loading, parsing, caching, etc. was redundant (you could instead import that thing directly from it's module), or they re-export multiple things which means extra junk gets pulled in when you only intend to import a selection of the exports. If you import the multiple things directly from their source modules, you are skipping the extra index module.

It's just a fact of reality that the most efficient module design is single export modules that are always deep imported as needed. Polling the community for their opinion on module design is just a test on their level of understanding, which in my years of arguing with the community about the index module anti pattern is extremely shallow. People tend to militantly vote for whatever is most familiar to their pre-existing habits.

IMO, Deno std needs to be totally refactored to one default export per module. It would be very nice if each of these modules had a sibling *.test.* file so tests can be specifically associated with each part of the API instead of mixed together in big test modules.

Something else really important is Deno std modules should be JavaScript with TypeScript JSDoc comments in .mjs modules, instead of .ts modules. This way they can be used in browsers and even other server runtimes like Node.js without requiring a build step. People importing these modules in project .ts modules won't notice a difference; type safety will be just as good thanks to the TypeScript JSDoc comments.

@dsherret
Copy link
Member

@jaydenseric all default exports would be a very bad user experience, in my opinion, for reasons already outlined in articles against default exports. Also, deno_std in JavaScript would not be a great development experience. It would be trivial to make a server that transpiles to JS on the fly then import from that. The transpiling could also be built into deno.land (there's an issue open somewhere in its repo).

Either they only re-export one thing, in which case the extra index module loading, parsing, caching, etc. was redundant (you could instead import that thing directly from it's module), or they re-export multiple things which means extra junk gets pulled in when you only intend to import a selection of the exports.

In many cases this has a negligible impact on performance and doesn't matter. I wouldn't describe it as always an anti-pattern because sometimes it has positive effects.

@jaydenseric
Copy link
Contributor

all default exports would be a very bad user experience, in my opinion, for reasons already outlined in articles against default exports.

Not sure what reasons you are referring to. If you are referring to trouble around default exports and ESM/CJS interrop in Node.js/npm tooling, then that is not really relevant to the Deno ecosystem which is all ESM. There are benefits to using a default export instead of a single named export; namely it minifies smaller. If you really hate default import syntax, you can still avoid the index module anti pattern by using a named import. The main thing is that the module exports only one thing.

Also, deno_std in JavaScript would not be a great development experience.

Do you mean the development experience for Deno std maintainers, or for Deno std users? Deno std users get the same type safety, they can't tell the difference; their development experience is just as great. Deno std maintainers need to learn the skills necessary to make the std lib as technically elegant, efficient, and usable as possible. It's pretty easy to write types in JSDoc comments instead of using TS proprietary syntax; I've written perhaps hundreds or (even thousands? IDK) of modules now typed using JSDoc comments and in some cases it's simpler than using TS syntax. Please consider the technical merits of an idea, instead of just deciding based on what is more familiar to you personally.

It would be trivial to make a server that transpiles to JS on the fly then import from that.

You're trivialising the complexity and inefficiency of making users everywhere setup servers to transpile on the fly. Using std lib in browser environment is not an edge case; it would happen a lot more if it were easily possible. For example, I wanted to use deferred in a Puppeteer browser environment for some unit tests of Ruck functionality intended for browser side use:

https://github.com/jaydenseric/ruck/blob/ce189ff2403690d9d8d89f1dd46a42ec73e6ffd2/scrollToHash.test.mjs#L79-L83

I was forced to write and maintain my own JavaScript module because the Deno std one was TypeScript that would not run in the browser environment:

https://github.com/jaydenseric/ruck/blob/f0d6afd90202d87b91ba52780ba7328a33bb60d9/test/Defer.mjs#L1-L20

The transpiling could also be built into deno.land (there's an issue open somewhere in its repo).

Complexity on top of complexity, for what? Just write JavaScript from the start; it's still type safe. There are downsides to working with messy machine transpiled code, and you are abstracting end users from the Deno std source code if they are debugging their apps and what to contribute a fix or improvement. You introduce complexity like, do you serve source maps?

In many cases this has a negligible impact on performance and doesn't matter.

Caching a bunch of crap bloating the resource requirements for deployments, the size of the cache on the hard drive, the time it takes to fetch cache on slow internet, the extra cache CLI output, etc. is bad enough when considering a Deno environment. But in a browser environment, index modules profoundly cause loading waterfalls and bloated performance.

I wouldn't describe it as always an anti-pattern because sometimes it has positive effects.

Very rarely it has a positive effect. I suppose you mean, by coincidence some modules are pulled in and cached ahead of time from when it would be discovered they are actually needed somewhere else in the module graph? Sheer luck like that is outweighed by all the extra modules you are pulling in. We have to optimise for what is more efficient on average.

Single export modules that are only ever deep imported as needed is a very efficient pattern that is extremely easy to follow. You don't need to paralyse yourself thinking about how to arbitrarily group things into index modules, and consider how wasteful it might be for what type of consumers in what types of situations.

@jaydenseric
Copy link
Contributor

jaydenseric commented Jun 16, 2022

How many tests in Deno projects use every kind of assertion? Most use 1, 2 or maybe 3. Currently we are forced to import from std/testing/asserts.ts and pull in literally every kind of assertion there is (along with all of their supporting helpers and dependencies). In most cases users would actually write less code to just get the thing they need...

https://github.com/jaydenseric/ruck/blob/f0d6afd90202d87b91ba52780ba7328a33bb60d9/serve.test.mjs#L3

Before:

Screen Shot 2022-06-16 at 9 57 37 am

After:

Screen Shot 2022-06-16 at 9 55 44 am

Another example…

https://github.com/jaydenseric/ruck/blob/f0d6afd90202d87b91ba52780ba7328a33bb60d9/HeadManager.test.mjs#L5-L9

Before:

Screen Shot 2022-06-16 at 9 59 20 am

After:

Screen Shot 2022-06-16 at 10 00 13 am

@dsherret
Copy link
Member

dsherret commented Jun 16, 2022

Not sure what reasons you are referring to.

I'm going to skip explaining default imports vs named imports because it's off topic.

It's pretty easy to write types in JSDoc comments instead of using TS proprietary syntax; I've written perhaps hundreds or (even thousands? IDK) of modules now typed using JSDoc comments and in some cases it's simpler than using TS syntax. Please consider the technical merits of an idea, instead of just deciding based on what is more familiar to you personally.

It's a little rude to say I haven't considered the technical merits or that I'm deciding based on what is familiar to me personally. How do you know this or what's familiar to me?

In my opinion, it's a much better development experience programming deno_std in TypeScript and I have had experience using JS docs and it's a good experience, but not as great of an experience as using TypeScript. Anyway, I'm not going to get into it because this is off topic.

You're trivialising the complexity and inefficiency of making users everywhere setup servers to transpile on the fly.

Couple of accusations in this post 😉. I don't mean making users setup servers to transpile on the fly, but rather deno.land having the ability to serve JS rather than TypeScript. This would benefit more than just deno_std, but also all modules served on deno.land.

But in a browser environment, index modules profoundly cause loading waterfalls and bloated performance.

Lots of files cause bloated performance in a browser because many requests need to be made. That's not necessarily a mod.ts problem. You could have the same code with a mod.ts and two files and then other code with no mod.ts and 10 files. The one with 10 files would load slower if you import all of them. Ideally for a browser you would take both scenarios then tree shake and bundle them so they would be equivalent.

Currently we are forced to import from std/testing/asserts.ts and pull in literally every kind of assertion there is (along with all of their supporting helpers and dependencies)

Ironically, if we took your suggestion and spilt this up into many individual files it would be slower out of the box in a browser environment for people to use than all of them together in one file because more requests would need to be made.

Very rarely it has a positive effect. I suppose you mean, by coincidence some modules are pulled in and cached ahead of time from when it would be discovered they are actually needed somewhere else in the module graph?

There are more considerations than performance or how Deno modules can load fastest in a browser. Structuring code for something that can be easily optimized by a bundler is a waste of time IMO. I think modules should be structured for developer experience, ease of use, and reusability.

The bottom line is this related to this PR (and we should probably split this up into a separate discussion if going to other topics like the ones gone into): for std/collections/, we have a mod.ts that you can choose to import if you don't care and want everything and we have individual files that you can choose to import if you just want one thing. For other folders in deno_std we should probably do the same to give the users choice over what they want to use and reduce bloat in a Deno application's module graph.

@jaydenseric
Copy link
Contributor

jaydenseric commented Jun 16, 2022

Please don't take any of the things I'm saying personally, I'm simply making an appeal to discuss concrete technical merits of ideas instead of making decisions based upon habits and what is familiar because based on prior experience people instinctively shut down discussion about module structure and format almost immediately on those grounds. If you are already considering the technical reasons, great. It's hard to tell from the outside that's the case though when things like "In my opinion, it's a much better development experience …" are being said without sharing what the specific technical arguments are.

Lots of files cause bloated performance in a browser because many requests need to be made. That's not necessarily a mod.ts problem. You could have the same code with a mod.ts and two files and then other code with no mod.ts and 10 files. The one with 10 files would load slower if you import all of them.

But it is a "mod.ts" problem. If you imported those 10 modules directly in your app's module, the browser would download them all in parallel and start entering them looking for more dependencies as they are coming in. The time is pretty similar if your app module imports one, or 10 modules. If you were to instead import those 10 modules via a mod.js middleman, then the browser has to wait for mod.js to download first before it can start work on the 10 module dependencies. You introduce a waterfall step that pushes the timeline out. Now imagine if everything in a complex app's dependency graph did stuff like that.

Regarding the overall number of modules that get pulled in, you would much rather lots of files with 100% of the code being pulled in being used ("hot") than having lots of files anyway because one of the re-exported things you don't need that was in an index module pulled in a whole other module graph that is 100% unused ("cold").

I've thought about this problem deeply for many years, and solved lots of problems relating to this optimising real world Node.js and Deno projects/packages, both bundled and buildless. So I have a lot to share on the topic and could respond to all of your points in great detail, but agreed it's better to do so in dedicated issues. In particular this statement solicits a lengthy response:

Structuring code for something that can be easily optimized by a bundler is a waste of time IMO. I think modules should be structured for developer experience, ease of use, and reusability.

This however would be a great start:

For other folders in deno_std we should probably do the same to give the users choice over what they want to use and reduce bloat in a Deno application's module graph.

A pleasant side effect would be that the std codebase will be some much more navigable if each public function had it's own module and tests specific to it in a sibling test file. It slowed me down trying to figure out the arbitrary placement of intermingled tests when contributing https://github.com/denoland/deno_std/pull/2124/files .

@albnnc
Copy link

albnnc commented Jun 18, 2022

Well, sorry for bumping up potentially closed discussion, but I want to put in my dime here.

Structuring code for something that can be easily optimized by a bundler is a waste of time IMO. I think modules should be structured for developer experience, ease of use, and reusability.

I really agree with this point and structure my code according to this idea for years. Of course, someone could say that I am wrong by doing so, but, as we can see, this is at least a topic for discussion.

It feels to me that it is good to have different ways to do same things, because you can pick the one that is better for you. You know, people have an ability to think. If the topic of discussion doesn't lead to thousands of legs being shot, maybe it's better not to take toys away from people?

I also think that the potential dependency graph bloating is not a leg being shot. Sure, it's not good to pull everything from a lib if you use a small portion of its code. But I think that there nothing wrong with it if you know what you're doing – you can use bundler with tree-shaking, or maybe you simply don't care.

What we have now is a screaming warning in code that simply enforces me not to structure my code in the way I am comfortable with.

In particular this statement solicits a lengthy response

This is why I would be happy and thankful to see this response.

Again, I'm not saying that mod.ts is a single way to go. But it is, in my opinion, is a definitely needed possibility that shouldn't be marked as "bad".

@jaydenseric
Copy link
Contributor

@albnnc reflect upon the wording of your comment and realise it could be summed up "I know my habits around importing are bad, but please allow me to do the wrong thing because It's what I'm used to". You have not put forward any technical arguments.

I also think that the potential dependency graph bloating is not a leg being shot.

It's the legs of the entire Deno ecosystem being blown off with a cannon. Over time, Deno std is intended and likely to be a production dependency of perhaps half of all public packages. If we give package authors the ability to import from std the inefficient way (mod.ts), experience shows most will use that out of habit and ignorance of the consequences. What this means, is if you want to write your package or project efficiently and only deep import what you actually need to load, cache, parse, and run, all that work is for nothing as soon as one of you dependencies loads from mod.ts. There is nothing you can do to optimise your app at that point, unless you PR that dependency to use the more efficient import structure. I have been dealing with this in npm land for many years.

Think of the nightmare lodash has been. All the devs got used to adding lodash as a dependency, installing a massive amount of modules to your local disk. Then they import just one or two functions from the main index in their code. I've been paid handsomely to help teams clean up multiple megabyte app bundles that got so bloated in part because multiple versions of all of loadash kept getting pulled in, often by project code, but sometimes by a dependency of a dependency of a dependency. Everyone competent knows now that for the love of all that is holy, deep import only the lodash function you need, and preferably from an ESM version of lodash. There are ESLint plugins that enforce this, that a lot of teams are using now. This whole nightmare affecting billions of internet users and thousands of developers, causing God knowns how many extra greenhouse gas emissions from straining 3g and hot phones, could have been avoided in the first place if lodash ONLY offered their functions as deep imports.

Deno std has some overlap in purpose with lodash; learn from it and don't repeat their mistake.

Users will not complain if an API is documented to only have deep imports. They will accept it and move on, and the Deno ecosystem will be so much healthier! They will make a little noise if we deprecate and eventually remove the existing mod.ts modules, but honestly who cares. It's just loud noises; they have a clear migration path and will move on. In 10 years time no one will know or care that the early std module structure was suboptimal and was changed. I am genuinely afraid of a future where Deno std has index modules; the idea of it makes me want to just quit and be a goat herder on a mountain somewhere.

@dsherret
Copy link
Member

You have not put forward any technical arguments.

The technical argument (repeated) is that the mod.ts file doesn't have any noticeable negative effects in many scenarios. Not everyone is using deno_std in the browser without bundling and tree shaking. In many scenarios using a mod.ts provides a better development experience when using an LSP because once you have the mod.ts import you can use auto-complete to add a named import. This provides better discoverability when hacking a way at a script. It's true that we could improve the LSP to give auto-complete for this, but currently it doesn't work that way and there doesn't seem to be a desire to add functionality to the LSP where it offers suggestions on these modules.

In my opinion, using a collections/mod.ts file in a quick data wrangling Deno script is a much better development experience than having to import each function in separate import declarations for the reason above and also to reduce verbosity. Please explain why it's better for someone writing a quick throwaway deno script to not use mod.ts? For example, this is the performance impact on my machine:

  1. Empty deno_dir, running app using collection/min_by.ts: 167ms, 6.14kb deno dir
  2. Empty deno_dir, running app using collection/mod.ts: 560ms, 359kb deno dir
  3. Existing deno_dir, running app using collections/min_by.ts: 60ms
  4. Existing deno_dir, running app using collections/mod.ts: 80ms

This "bloat" has no noticable negative impact in this scenario and contributes positively to a good LSP experience. It could also be argued that it's going to save me time now because I get auto-complete for other functions that are exported from mod.ts so I don't need to spend any time manually updating import declarations. I'm also probably discovering functions quicker because I don't need to leave my editor to see what the possibilities std/collections offers me.

Think of the nightmare lodash has been. All the devs got used to adding lodash as a dependency, installing a massive amount of modules to your local disk.

Yeah, people who are distributing modules should be importing the individual modules directly from deno_std instead of importing from a mod.ts. Where it doesn't always matter is for end application developers and people writing quick deno scripts to do tasks on their system.

But it is a "mod.ts" problem. If you imported those 10 modules directly in your app's module, the browser would download them all in parallel and start entering them looking for more dependencies as they are coming in. The time is pretty similar if your app module imports one, or 10 modules. If you were to instead import those 10 modules via a mod.js middleman, then the browser has to wait for mod.js to download first before it can start work on the 10 module dependencies. You introduce a waterfall step that pushes the timeline out. Now imagine if everything in a complex app's dependency graph did stuff like that.

Disagree, this isn't necessarily a "mod.ts" problem. In the 10 module scenario you could have separate files with common code that are shared between modules. The waterfall problem would still exist in that scenario, but if you bundle + treeshake + minify then the problem goes way. It's better for these tools to deal with this problem space rather than introducing this consideration to the codebase which is guaranteed to not be as optimized as what these tools can do.

@jaydenseric
Copy link
Contributor

jaydenseric commented Jun 19, 2022

The technical argument (repeated) is that the mod.ts file doesn't have any noticeable negative effects in many scenarios.

Literally every single scenario is faster when using deep imports instead of index module. In poor hardware environments, the TypeScript type checking is faster if it has a lot less code to deal with. Why make the performance worse for CI, people with poor hardware machines, etc. if it is entirely avoidable? The cache size is smaller. This means less noise when the Deno CLI is listing all the things it's caching, it means less space on disk. It's not just about the cache size about using one version of one of the smallest Deno std modules; in a realistic module graph there might be 5 or more versions of Deno std being pulled in and all of those kb extra of modules stack up. Also, we are not just discussing the cost of mod.ts for the lightest of std modules, but rather all of Deno std modules, some of which might be a lot heaver than others.

Not everyone is using deno_std in the browser without bundling and tree shaking.

As an engineering philosophy, Deno should strive to enable quality results without a build step wherever possible. If the same result can be achieved without a build/compilation step, that is inherently better. We should only build/compile/transpile in circumstances we can't get the result we need any other way.

It's alarming core Deno team members are prepared to completely destroy the usefulness of the Deno ecosystem for those of us who want to use native ESM modules in the browser without build steps. I had high hopes for Deno having a philosophy of reducing the need for boilerplate and complicated build tooling to get anything done; being a server environment that implements all the web APIs we need to make truly isomorphic applications. It seems Deno is hell bent on forcing everyone into complicated build tooling; npm ecosystem and complexity all over again.

I spent a lot of time and effort to create and publish https://ruck.tech to prove that not only are buildless React web applications possible, but (in my strongly defensible opinion) better than any frameworks relying on bundling that came before.

Regardless of how great buildless is, using deep imports instead of index modules will greatly improve the situation for those of us who still like to bundle. Deep imports are like tree shaking, before the bundler even runs. It will always be more efficient and lead to more predictable bundling, with faster build times. There are no dilemmas over what is safe to tree-shake at build time due to potential module side effects. This has greatly complicated webpack and esbuild and other bundlers. Just take a look at https://webpack.js.org/guides/tree-shaking/#mark-the-file-as-side-effect-free and tell me; does that match the Deno philosophies of simplicity, ease of use, and technical elegance?

In many scenarios using a mod.ts provides a better development experience when using an LSP because once you have the mod.ts import you can use auto-complete to add a named import.

Deep imports have do have intellisense:

Screen Shot 2022-06-19 at 9 13 36 am

If that intellisense can be even richer, great. Lets work on that on the Deno side. We need to be building the tools we need to write efficient code, not the opposite. We should not write poor code to avoid aesthetic limitations of an IDE.

Yeah, people who are distributing modules should be importing the individual modules directly from deno_std instead of importing from a mod.ts.

I'm really happy you've come around to that. Currently, it's impossible for us package authors to do that as mod.ts is the only way std modules can be imported.

Disagree, this isn't necessarily a "mod.ts" problem. In the 10 module scenario you could have separate files with common code that are shared between modules. The waterfall problem would still exist in that scenario, but if you bundle + treeshake + minify then the problem goes way.

I'm not following. I explained a specific situation where a waterfall would exist. It doesn't matter what is going on in those 10 modules or what other common code they import; if you put a mod.ts above them and import that it would be an additional step in the timeline compared to just directly importing the 10 modules and skipping the mod.ts layer.

I suppose you are alluding to the idea that by accident, for some people, in some apps, that import the exact right combination of things from some modules, having "common code" in a single module will not be harmful? This is Deno std we are talking about, millions of people will 100% experience bloat if mod.ts exists. That outweighs the few people that happen to benefit from a coincidence.

… if you bundle + treeshake + minify then the problem goes way. It's better for these tools to deal with this problem space rather than introducing this consideration to the codebase which is guaranteed to not be as optimized as what these tools can do.

Literally impossible. Deep importing is 100% perfect tree shaking ahead of time; you can only hope a bundler is able to try to untangle index modules to a result anywhere near what you would get for free if you actually wrote the intended code in the first place.

People like me that are only interested in using Deno without bundlers deserve an efficient ecosystem. I'm not going to introduce a bundler in my Ruck projects if I encounter a front end dependency that imports a Deno std module via mod.ts; the entire framework of Ruck is incompatible with the concept of bundles. It's not possible.

@jaydenseric
Copy link
Contributor

jaydenseric commented Jun 19, 2022

It's helpful to the discussion to clarify terms, as there is a subtle difference between "index" modules and "commons" modules.

Index module

The publisher (i.e. x):

// x/index.mjs (or x/mod.ts, the concept is the same)
export { default as aaaaa } from "./aaaaa.mjs";
export { default as bbbbb } from "./bbbbb.mjs";
export { default as ccccc } from "./ccccc.mjs";

This "index module" pattern is technically inferior in every scenario.

Consumer example:

import { aaaaa, bbbbb, ccccc } from "x/index.mjs";

This would have skipped a waterfall step and loaded faster if the desired modules were imported directly:

import aaaaa from "x/aaaaa.mjs";
import bbbbb from "x/bbbbb.mjs";
import ccccc from "x/ccccc.mjs";

There is no technical downside to using these deep imports instead, except the pre min+gzip byte count from the consumer project import statements has grown from 50 bytes to 98 bytes. This is still a massive bytes saving overall though, as the 143 byte x/index.mjs module was entirely skipped.

In a buildless project (or when considering the speed of Deno caching), the extra bytes downloaded up front is significantly faster than the alternative of loading the extra x/index.mjs module in a waterfall step, before the deeper modules could be detected and start loading.

If the consumer project actually had a lot of the same deep imports repeated across many modules, and assuming (in the case of a web app) that a single user would load all those modules as the result of one request (which they might not if they are only visiting one route worth of components), then the increase in byte size for the deep import statements might become relevant. But, if you do have gzip or brotli compression for the served modules the keyword repetition is mitigated. Also, if you minify, the default imports (unlike the named imports) can be compressed to:

import a from "x/aaaaa.mjs";
import b from "x/bbbbb.mjs";
import c from "x/ccccc.mjs";

Now the main difference is the specifiers. This also goes away if you bundle, as the specifiers disappear in the process.

Commons module

The publisher (i.e. x):

// x/mod.mjs (or x/mod.ts, the concept is the same)
export function aaaaa () {}
export function bbbbb () {}
export function ccccc () {}

Consumer example:

import { aaaaa, bbbbb, ccccc } from "x/mod.mjs";

By chance, the user has used 100% of what is exported so there is no waste.

For this example:

import { aaaaa } from "x/mod.mjs";

There is waste because bbbbb and ccccc exports were undesired, but had to be downloaded regardless. There is nothing a buildless project can do to mitigate this waste. A project that bundles still has to work harder during the build to attempt to remove the waste. There is a massive amount of waste problems generally associated with bundles though, which is a large topic for another discussion (Next.js route bundles for example contain duplicated code, as not everything can have it's own app level commons bundle).

Mixed module

You can also have a combination of an "index" and "commons" module.

The publisher (i.e. x):

// x/mod.mjs (or x/mod.ts, the concept is the same)
export { default as aaaaa } from "./aaaaa.mjs";
export function bbbbb () {}
export function ccccc () {}

This suffers the downsides of both approaches at once.

As you can see, while an "index" is always wasteful, at least if the entire project module graph is diligent in avoiding it the waste can be avoided. In contrast, a "commons" module has at least some scenarios in which 100% of the exports happen to be used without waste, but it guarantees unavoidable waste in all other scenarios.

@dsherret
Copy link
Member

Literally every single scenario is faster when using deep imports instead of index module. In poor hardware environments, the TypeScript type checking is faster if it has a lot less code to deal with. Why make the performance worse for CI, people with poor hardware machines, etc. if it is entirely avoidable? The cache size is smaller. This means less noise when the Deno CLI is listing all the things it's caching, it means less space on disk.

This is a choice the consumer should make. In the scenario above I demonstrated, as the consumer I chose to import mod.ts and it cost me 20ms of startup time per run while I was writing a throwaway hacky script. I don't care about 20ms in my scenario.

The argument I'm making is that it should be the consumer that chooses what they want.

As an engineering philosophy, Deno should strive to enable quality results without a build step wherever possible. If the same result can be achieved without a build/compilation step, that is inherently better. We should only build/compile/transpile in circumstances we can't get the result we need any other way.

Again, consumer can choose. In my scenario of just hacking out a script, I don't care about 20ms of savings or my cache using a few more kb.

It's alarming core Deno team members are prepared to completely destroy the usefulness of the Deno ecosystem for those of us who want to use native ESM modules in the browser without build steps.

Again, the consumer can choose, but even with choosing not to use a mod.ts file, deno std importing from internal modules will cause a waterfall problem that could be eliminated by using a bundler. Also, using a bundler does not necessarily mean you need to introduce a build step because the server could bundle once on startup and cache the result.

Deep imports have do have intellisense.

I mean auto-imports and not registry completions.

If that intellisense can be even richer, great. Lets work on that on the Deno side. We need to be building the tools we need to write efficient code, not the opposite. We should not write poor code to avoid aesthetic limitations of an IDE.

It's a little questionable if it should exist though because most modules distributed only expect a mod.ts file to export the public api and doing this might start giving auto-completions for internal APIs. We should explore this more though... ex. the registry could have the opinion that if there's no mod.ts file then it could suggest giving auto-imports for other modules in the directory (I forget exactly how all this works under the hood though). Until then, I think we should have mod.ts files.

I'm not following. I explained a specific situation where a waterfall would exist. It doesn't matter what is going on in those 10 modules or what other common code they import; if you put a mod.ts above them and import that it would be an additional step in the timeline compared to just directly importing the 10 modules and skipping the mod.ts layer

Sorry, I forgot what I wrote initially and what I mean here is not necessarily importing 10 files directly (which even though it's parallel could load slower than just one file in many scenarios because it's more overhead and there could be a limit to the parallelization below 10). What I mean, is if the consumer imports module A and module A depends on an "internal" module B, then the waterfall problem will still exist, but that would go away with a bundler. It could also be changed by the two modules being combined together, but again though, in my opinion, the internal module structure of a codebase shouldn't be concerned with reducing waterfall module loading scenarios.

@jaydenseric
Copy link
Contributor

jaydenseric commented Jun 19, 2022

The argument I'm making is that it should be the consumer that chooses what they want.

I've already said, that as sure as the sun rises if you give the public the choice they will do it the wrong way everywhere and ruin the ecosystem.

People learning JavaScript always complain that there are many ways to do things, and it's hard for them to tell the right way. Consumers don't want "choice". There clearly is a right choice, and a wrong choice.

In my scenario of just hacking out a script, I don't care about 20ms of savings or my cache using a few more kb.

What makes you think people couldn't hack out scripts just as fast if there was only one way to import from std? You're presenting a false dilemma. We can have our cake and eat it; hack things easily, and have perfect technical efficiency.

I mean auto-imports and not registry completions.

In 10 years I have never used or cared about auto imports. But as has already been discussed; the auto imports could be made to work with deep imports if they don't already.

the internal module structure of a codebase shouldn't be concerned with reducing waterfall module loading scenarios.

I am in utter shock you would say that. What can I say. You wilfully don't care about web performance or web apps or even speedy deno caching. Is there anything I can say that could make you start to care about performance? I'm trying really hard to have a good faith discussion. Maybe Deno is not for me. Can someone else at Deno confirm if this is the official Deno position?

@dsherret
Copy link
Member

dsherret commented Jun 19, 2022

I don't think this is a hard choice. One way is to get everything in a folder and the other way is to get specific parts of the folder. Sometimes that matters and sometimes that doesn't (ex. adds 20ms and under 1mb to my cache in the scenario I demonstrated above).

I am in utter shock you would say that. What can I say. You wilfully don't care about web performance or web apps or even speedy deno caching.

What's so shocking? Refactoring out code to a common module is a very common coding practice to encourage code reuse and to make the code cleaner. Internal code maintainability shouldn't be sacrificed to prevent waterfalling since people should use automated tools that distribute the code bundled (again, this does not require a build step because it can be done JIT).

Also, the scenario I'm suggesting of using a bundler would lead to speedier web performance.

Is there anything I can say that could make you start to care about performance? I'm trying really hard to have a good faith discussion.

I don't care about 20ms of startup time and under 1mb of cache use in the scenario I presented.

Maybe Deno is not for me. Can someone else at Deno confirm if this is the official Deno position?

No. None of this is the official Deno position—these are my personal thoughts.

@albnnc
Copy link

albnnc commented Jun 19, 2022

reflect upon the wording of your comment and realise it could be summed up "I know my habits around importing are bad, but please allow me to do the wrong thing because It's what I'm used to". You have not put forward any technical arguments.

You've misunderstood everyhing. None of these habits are bad, because, as me and @dsherret keep saying, in certain scenarios your points simply don't work.

experience shows most will use that out of habit and ignorance of the consequences

If this experience is valid, it is not a problem of mod.ts, since people should just choose a variant which works for them. Maybe we should hint different variants in README, for example. For someone mod.ts is a way to go – not because of bad habits, but because they prefer cleaner / smaller source code and bundler usage.

If you care about it that much, then maybe you should spend more time on giving lectures, writing blogs, etc. Educate people, don't take toys from them.

Think of the nightmare lodash has been.

Completely wrong argument. lodash was an excelent library for its time. Talking about module bloating, yes, it's a big mess now, but the main problem is that there was no clear way to import a part of lodash. It was not designed for that usage way, but deno/std is (partly for now), and it would be good to have an ability to choose.

Everyone competent knows now that for the love of all that is holy, deep import only the lodash function you need, and preferably from an ESM version of lodash.

Everyone competent knows that ESM version of lodash works not so well, because many of library functions depend on other library functions – more than probably should. Simple merge function pulls too many space, if you care. Hacky solutions like compile-time exclusions don't help much. lodash simply not designed for that kind of usage.

Users will not complain if an API is documented to only have deep imports.

Maybe and maybe not. Stop thinking for others.

Literally every single scenario is faster when using deep imports instead of index module.

I don't care, since I use bundler. And I know many devs which use it too.

As an engineering philosophy, Deno should strive to enable quality results without a build step wherever possible.

Why? Deno is a platform that might host many different projects, you simply have no way to know every case. Of course, you shouldn't do so, but the case with bundler is quite common and easy to understand.

If the same result can be achieved without a build/compilation step, that is inherently better.

Disagree, since the code readability, in general, is worse. You're writing more to import every deep import directly.

We should only build/compile/transpile in circumstances we can't get the result we need any other way.

Just tree-shake your code while bundling it. Even if Deno CLI tooling doesn't do it (for now), it's relatively easy to implement on your own with other tools.

It's alarming core Deno team members are prepared to completely destroy the usefulness of the Deno ecosystem for those of us who want to use native ESM modules in the browser without build steps.

You are just blaming people for letting others to choose. Wake up and stop thinking that your point is the only correct. Noone frobidding you from importing direct .ts files.

Talking about assertions module, yes, I agree that it would be better if it was split into multiple modules. But it also could include mod.ts to import them all – for the ones that want it.

I spent a lot of time and effort to create and publish https://ruck.tech to prove that not only are buildless React web applications possible, but (in my strongly defensible opinion) better than any frameworks relying on bundling that came before.

You are a very experienced developer, we got it. But even if buildless variant is faster in any way than the one with a build step, you can't say that it is better. Because, you can't cover everything up with your specific example (or even multiple examples) – that's sophism.

We need to be building the tools we need to write efficient code, not the opposite.

Tools could be different and sometimes you should have brain to use them. If you don't, that's not a tool problem.

I explained a specific situation where a waterfall would exist.

And there situations where it would not. Nothing wrong with it.

People like me that are only interested in using Deno without bundlers deserve an efficient ecosystem.

And people like me that are interested in writing code with better readability while bundling deserve a handy ecosystem too. Again, stop pulling the blanket.

There is nothing a buildless project can do to mitigate this waste.

Then simply don't use mod.ts.

This is a choice the consumer should make.

Agree.

I've already said, that as sure as the sun rises if you give the public the choice they will do it the wrong way everywhere and ruin the ecosystem.

Why do you think that everyone is stupid? Sure, there will be some bad examples, but the community is evolving and, in general, people will reject "bad" modules / libraries. If someone fails to pick the "right" way, then he will learn and try later. I'm quoting "right", since, as I keep pushing, there is no a single right way.

You can say that something is bad if you have arguments about this specific case. But you can't pick the only right solution until you disprove others.

cjihrig pushed a commit to bartlomieju/deno_std that referenced this pull request Jun 28, 2022
Using collections/mod.ts will pull in a whole bunch of unrelated code.
We should not have anti-patterns like this in std.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants