-
Notifications
You must be signed in to change notification settings - Fork 641
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
Conversation
Using collections/mod.ts will pull in a whole bunch of unrelated code. We should not have anti-patterns like this in std.
|
I'm in favor of removing |
Opnions from discoard:
|
There was a problem hiding this 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
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. |
Index modules that export things from other modules ( 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 Something else really important is Deno |
@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).
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. |
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.
Do you mean the development experience for Deno
You're trivialising the complexity and inefficiency of making users everywhere setup servers to transpile on the fly. Using I was forced to write and maintain my own JavaScript module because the Deno
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
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.
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. |
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 https://github.com/jaydenseric/ruck/blob/f0d6afd90202d87b91ba52780ba7328a33bb60d9/serve.test.mjs#L3 Before: After: Another example… Before: After: |
I'm going to skip explaining default imports vs named imports because it's off topic.
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.
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.
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.
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.
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. |
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.
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 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:
This however would be a great start:
A pleasant side effect would be that the |
Well, sorry for bumping up potentially closed discussion, but I want to put in my dime here.
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.
This is why I would be happy and thankful to see this response. Again, I'm not saying that |
@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.
It's the legs of the entire Deno ecosystem being blown off with a cannon. Over time, Deno Think of the nightmare Deno 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 |
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:
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.
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.
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. |
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
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?
Deep imports have do have intellisense: 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.
I'm really happy you've come around to that. Currently, it's impossible for us package authors to do that as
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 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
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 |
It's helpful to the discussion to clarify terms, as there is a subtle difference between "index" modules and "commons" modules. Index moduleThe publisher (i.e. // 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 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 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 moduleThe publisher (i.e. // 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 Mixed moduleYou can also have a combination of an "index" and "commons" module. The publisher (i.e. // 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. |
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.
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.
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.
I mean auto-imports and not registry completions.
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.
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. |
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.
What makes you think people couldn't hack out scripts just as fast if there was only one way to import from
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.
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? |
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).
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.
I don't care about 20ms of startup time and under 1mb of cache use in the scenario I presented.
No. None of this is the official Deno position—these are my personal thoughts. |
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.
If this experience is valid, it is not a problem of 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.
Completely wrong argument.
Everyone competent knows that ESM version of
Maybe and maybe not. Stop thinking for others.
I don't care, since I use bundler. And I know many devs which use it too.
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.
Disagree, since the code readability, in general, is worse. You're writing more to import every deep import directly.
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.
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 Talking about assertions module, yes, I agree that it would be better if it was split into multiple modules. But it also could include
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.
Tools could be different and sometimes you should have brain to use them. If you don't, that's not a tool problem.
And there situations where it would not. Nothing wrong with it.
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.
Then simply don't use
Agree.
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. |
Using collections/mod.ts will pull in a whole bunch of unrelated code. We should not have anti-patterns like this in std.
Using collections/mod.ts will pull in a whole bunch of unrelated code.
We should not have anti-patterns like this in std.