-
Notifications
You must be signed in to change notification settings - Fork 875
StyleGuide 04-10 - Barrels #1301
Comments
I just found this out the hard way, after I went through all my code and added barrel imports everywhere, according to the Angular2 Style Guide. I guess I'm going with the '../shared/index' approach for now, but it takes away a bit of the point of barrel imports (shorter, simpler import lines). |
@johnpapa I'm curious if you've seen this behavior also. I'm adding the cookbook recipe for barrels and running into this same behavior. It seems that the only solution is to add |
It looks like the angular-cli generators add the barrel file to the |
Seems like a poor design decision honestly. |
We are going to move this recommendation from The CLI adds the entries for yourself and Webpack support this out of the box. |
Here is some reasoning here on the problem we are addressing. The number of times we import something is very large in an app. We write For example, here is how we would import these 4 services in a shared folder without any barrels. Notice spinner and toast are in their own nested folders under shared, too.
This is fairly common and the larger an app gets, the more imports we have. Can we do better? If we add barrels with the
That is shorter ... but what if we now use barrels to re-export form the nested folders' barrels too? Then we can make it a single line like this vs
And if we are willing to add a map for the folders in vs
And if we use the CLI it adds the map for us. The only Do here is be consistent with whatever technique you want (which will be added to the guide). In short:
|
I agree, John. It's too bad that SystemJs doesn't have an option recognize On a related note, when a folder only contains a single exportable item, perhaps making a package out of that folder should be a Consider since it really is just changing your import from something like |
I like all your points there @jmcooper. One of the reasons of doing a barrel for a single export is for consistency as you said. It is better not to have some imports on folder and some imports on direct files. |
The real issue here is to see if there is a way to easily omit @Foxandxss do you agree? if so, can you make the change and we can close this? |
@bodiddlie there's an open issue related to this topic in the SystemJS issue tracker systemjs/systemjs#1164 |
just including a function that goes through the sub folders in |
I have a big problem with barrels in its current form: The order seems to matter. There's an issue on stackoverflow as well. For example, if I have a barrel like this: export * from 'my.component' // using my.service via DI
export * from 'my.service' Then I get an error export * from 'my.service'
export * from 'my.component' // using my.service via DI I see this as problematic because:
|
@chapati23 I suppose this issue is more related to |
thanks @mgechev, i've read the forwardRef docs as well as the respective DI cookbook paragraph. I don't understand this 100% but I guess if |
It's better to move this discussion to gitter. |
@chapati23 inside |
that's what I ended up doing, @awerlang. and that's exactly where I feel it should be communicated more clearly what barrels are for and what barrels are not for. because in my last react-project and in the node-project before that we used barrels internally as well which (at least for node) seems to be a common practice. so i think a little explanation box with some caveats on barrels wouldn't hurt /edit: general barrel-order issue: |
I wonder if 4-10 should be removed entirely as a recommendation. I would expect the recommendations in the documentation to represent a "golden path". So it's surprising that by using the loader recommended in the QuickStart guide and carefully following the style guide's recommendations on directory structure, I end up with broken code. Shorter/fewer imports are nice, but to make barrel imports (as described in 4-10) work, I had to add more lines of code than I saved. Namely:
Even if it was netting out to less code, I don't think it would be worth the added complexity and fragility. Also, I believe the circular dependency issue is worse than #9344 suggests, in that it's not just a problem of sensitivity to ordering. This reddit post has a nice example of a false circular dependency that (AFAICT) will manifest regardless of any reordering of exports in the barrel files. |
This is going to be reviewed again soon when we update the style guide for RC5. |
FYI
Latest angular-cli does not create barrels anymore, too. |
1. i did not found any barrels in the latest angular-cli blueprints 2. see angular/angular.io#1301 (comment)
Unfortuantely I find ngModules a poor replacement for barrels (because ngModules != typescript modules). I have so far found the following tactic working well, when applied consistently:
This way allows me to think of the barell file like a *.d.ts file of the whole module. It has resolved the circular dependency issues I was having. I really like having a barrel file for shared modules as it makes my other code considerably cleaner. YMMV. |
|
.... aaaand to set up the DI! ;-) |
I understand as much, hence my suggestion for working with barrel files while avoiding some common associated issues. I think it's not a good idea to flat out discourage their use. |
I think I maybe late for the discussion, but I like barrels. Not only I like barrels, but Instead of You would Giving a folder structure like: src
|- app
|- models
|- components using models
|- ... somewhere you load the model It's easy to read, and easy to maintain. I know the feature can be use without barrels, but I didn't even try this with SystemJS, I don't know if this works with this, and I would say it does not. is just a Code Style, and it does not suppose to be followed blind, but it is really important for us as developers to have a consistent Code Style, and for this to evaluate the benefits about drop the suggestion of a useful feature. I think this is only a SystemJS bug, and it was being tracked, and ultimately fixed as systemjs/systemjs#1164 (But I don't know if it was really about loading barrel folders). At the end, I think that instead of dropping the use of barrels folders, you should consider to drop the suggestion of using SystemJS as a build system. But as I said, I don't know if it had being updated to APR an others TypeScript new features. |
@michaeljota We all like barrels. What has changed is that they are now considered a matter of personal style, not Angular style. There was a time when we thought they would be almost essential for aggregating access to components. After modules were introduced, that purpose melted away. They still have utility for aggregating model and service classes; I'm sure they are useful elsewhere too. SystemJS is not a factor in this decision. Barrels and SystemJS get along fine. Keep using barrels as you find them helpful. We have decided not to restore barrels to the Style Guide. This issue is closed. |
Since SystemJS doesn't use node module resolution, but angular2 (or rather the ts config we use) does, the barrel import syntax requires either changes to the imports, or adding entries in systemjs.config.js.
If you have an
app/shared/index.ts
folder and doimport { SharedComponent } from '../shared';
somewhere, the default SystemJS setup from the tutorials will end up throwing a 404 because it tries to load../shared.js
.If you change the import to be from
'../shared/index
, or addapp/shared
to the packageNames in systemjs.config.js, then it works.The text was updated successfully, but these errors were encountered: