avoid recursive imports for index

[einari]

Client had an import to index which in turn exports Client. This can cause some real weird errors.

[woksin]

There is nothing wrong with having Client import from '../index', it is the technique used all across this project. However the Client has references to LoggingBuilder and LoggingBuilder does not have dependencies on anything else from the '../index' module, hence it should be the first export in the index.ts file.

I cannot suggest the changes, but the fix would be for these to switch places

export { Client, ClientBuilder } from './Client';
export { LoggingBuilder, LoggingBuilderCallback, WinstonOptionsCallback } from './LoggingBuilder';

The dependencies and the order of when they are imported and exported is of high significance. The index files should serve as pure module definitions taking full control over the importing and exporting of modules

[einari]

I would argue it would be a good practice to within a module to include explicitly what it needs instead of going through index, that way you have full control of it all within the module.

[einari]

That would also give you the option of not having to export everything from the module.

[einari]

Adding to this a bit more; to me it seems that having to maintain the correct order in index for all scenarios within a module seems like a coupling that makes it harder to maintain, rather than being explicit about your imports. Then ordering doesn't matter. For circular dependencies, when those cases occurs - which I think are more of an edge case, we probably need to go through something like index or other composition.

[woksin]

I would argue it would be a good practice to within a module to include explicitly what it needs instead of going through index, that way you have full control of it all within the module.

I would argue the opposite, importing files directly gives you the least amount of control since you have no control over what the thing you import imports itself.

[woksin]

Adding to this a bit more; to me it seems that having to maintain the correct order in index for all scenarios within a module seems like a coupling that makes it harder to maintain, rather than being explicit about your imports. Then ordering doesn't matter. For circular dependencies, when those cases occurs - which I think are more of an edge case, we probably need to go through something like index or other composition.

From my experience and understanding the order does matter either way, and in that case it's my opinion that someone needs to take control over the importing. And yes, it certainly does not make it easier to maintain, but it makes it much easier to debug when your project has 100+ files and it builds but when you run the application and you suddenly get a Super expression must either be null or a function, not undefined or something obscure like that. Then it becomes hard to find and fix the problem.

I see writeups that tackles this problem by having these index and internal files that takes charge over the module importing and exporting https://medium.com/@daveperipatus/heres-how-to-scale-this-pattern-to-larger-projects-8227eb45e1cc

[woksin]

That would also give you the option of not having to export everything from the module.

There are patterns that eliminates this issue by having "internal" files that takes the importing / exporting responsibility and index files which represents the public interface of the modules

[woksin]

It's not like I like those patterns, in fact I do detest them 🤣

[einari]

The article proposes what I proposed:

The rules are this:

• add an index file per folder of files, which exports all files
• within a folder, you MUST import siblings directly
• outside a folder, you SHOULD import from the target folder’s index

Or am I not reading this right :)

[woksin]

Huh, maybe I read it wrong as well. I based it mostly of the parent article that's on top of that article which is the one I meant to copy 😄 . https://medium.com/visual-development/how-to-fix-nasty-circular-dependency-issues-once-and-for-all-in-javascript-typescript-a04c987cf0de

[woksin]

I don't really understand the seocond rule there, but if it works I won't complain. That gives us the benefit of not having to export everything publically.

Maybe we can try that strategy? It is pretty much what we're doing already, except from the import siblings directly part.

But either way, we should do the suffle on the export in the index file since given the ordering it makes sense the the LoggingBuilder is exported first

[einari]

I quite like those 3 simple rules. And yes, we need to keep an eye on ordering and keep them logically as dependencies.

Are we good with this with the reordering I've done then??

[jakhog]

I like those three rules, but also just to clarify (then we can put these rules into contributing guidelines at some point) - these rules apply internally in a package right?

And then when importing things from another package, do we allow importing from specific folders within the package, or just the package index?

About the order, I didn't think that it mattered as the modules are only loaded once? Or is this some way to avoid circular imports?

[einari]

I like the rules - they're simple and I think they cover 99% - circular dependencies is a nightmare, but I think that is more the edge case.

With those rules I don't think ordering matters for the most part.

I would say the rules are for within a package.
For consumers, they should rely on what is publically exported IMO. But for 3rd parties using our stuff, we can't enforce that in anyway.

It makes sense to have something like this in our contrib guidelines.

[woksin]

Looks good now

[jakhog]

I wasn't thinking about how we use external packages (as we don't always have a choice) - or how consumers use our packages. But most of our code is split over multiple packages, like the SDK.

[einari]

I think it makes sense to still be relying on what is exported from index for each level.
Not requiring to export everything at the root level.

[woksin]

I wasn't thinking about how we use external packages (as we don't always have a choice) - or how consumers use our packages. But most of our code is split over multiple packages, like the SDK.

We use yarn workspace for our development environment, but in the end these things are really independent packages published to npm and should be treated that way.

I think that when referencing from external modules should always just import the main index file of that package