angular / angular-cli

CLI tool for Angular
https://cli.angular.io
MIT License
26.73k stars 11.98k forks source link

Feature: add default export option when using a schematic to generate a component #25023

Closed brandonroberts closed 4 weeks ago

brandonroberts commented 1 year ago

Command

generate

Description

When I want to generate a component to use as a lazy loaded component with loadComponent I can use the schematic

ng generate component about --standalone
```ts
import { Component } from '@angular/core';
import { CommonModule } from '@angular/common';

@Component({
  selector: 'app-about',
  standalone: true,
  imports: [CommonModule],
  templateUrl: './about.component.html',
  styleUrls: ['./about.component.css']
})
export class AboutComponent {

}

Then when I go to use the component in the route config, I immediately get an error because its not a default export

[
  { path: 'about', loadComponent: () => import('./about/about.component') }
]

Describe the solution you'd like

Add an option to generate a component with the class as a default export

ng generate component about --standalone (--export-default or --default-export)
import { Component } from '@angular/core';
import { CommonModule } from '@angular/common';

@Component({
  selector: 'app-about',
  standalone: true,
  imports: [CommonModule],
  templateUrl: './about.component.html',
  styleUrls: ['./about.component.css']
})
export default class AboutComponent {

}

Describe alternatives you've considered

Manually update the component after generation

alan-agius4 commented 1 year ago

//cc @dgp1130 as he had some strong opinions about default exports in general as they go against G3 guidelines.

dgp1130 commented 1 year ago

The google3 TypeScript style guide generally disallows default exports and provides some justification. I think all of those points generally apply outside of Google as well. Angular style guide doesn't seem to provide any guidance here, but it does include the line "Do define one thing, such as a service or component, per file" with one reasoning for it being "A single component can be the default export for its file which facilitates lazy loading with the router", which at least tells me that default exports aren't disallowed.

Additionally external to google3 we have CommonJS interop, which is a major challenge for default exports. These days, Angular is all-in on ESM so that isn't a major issue, but if devs choose to use default exports in their application, it easily bleeds over into other CommonJS libraries they might be using and introduce those incompatibilities, so I still generally would not recommend default exports, solely for that reason.

That said, personally I'm ok with default exports when they avoid magic names. For example, if we had an API like loadComponent('foo') and under the hood that did import('foo').then((module) => doSomethingWith(module.someExport)), then I feel a default export is actually simplifying that experience and avoiding a requirement for the magic someExport name with special behavior. Of course we've generally avoided those kinds of designs for a number of reasons, the router being one notable area.

Recently we did add support for default exports in the router, so I can understand wanting to take advantage of that out of the box when generating a component. I can see some value to the idea that "all route components should use default exports".

However the core philosophical concern I have is that defining a component is independent of defining a route. The component decides whether or not it is the default export, yet the route is the one which is simplified by consuming a default exported component. It is entirely possible to have a component used as a route in one location, but then also used as a sub-component in another location, yet this presents a contradiction between "all route components should use default exports" and "all non-routes should use named exports", since the component is used in both contexts.

This makes me think that ng generate component is the wrong place for this kind of functionality. If we have some reasonable guarantee that a component is always used as a route (such as a home.route.ts file convention or a routes/ directory), then a default export might make sense. However ng generate component doesn't know that.

We've discussed adding ng generate route for standalone, which I think would generate the underlying component as well. I wonder if it would make more sense to use a default export there (whether by default or as an option) since it is very likely that this component will only be used as a route. I guess it might need to be plumbed through to the component schematic anyways to support that, but I think it would present a better experience if users don't need to think about it. I know I would never think to write --default-export in advance and would just find it easier to add the default keyword manually. But if we made ng generate route use a default export by default, users would not need to think about it or explicitly make that choice. Of course the trade-off here is that users might not understand why some components use default exports and why some don't, which can lead to general confusion.

I'm curious to hear @atscott, @alxhub, and @jelbourn's opinions on this. I think a lot of it comes down to:

  1. Do we want or intend to be more restrictive about default export in the Angular style guide?
  2. Do we want to encourage or recommend default export over named exports for routing use cases? a. If yes, are we ok with diverging from the Google style guide here? Should we explore making the same exception in the google3 style guide?
  3. Does this fit in ng generate component? Does it fit in ng generate route instead?

Ultimately, like most things with schematics this comes down to a style question, and the answer to that question is what should inform what we support in schematics.

jelbourn commented 1 year ago

Do we want or intend to be more restrictive about default export in the Angular style guide?

I think this is mostly a CLI question (aka a Doug question) since it most heavily impacts the build and bundling aspect of Angular rather than any framework concepts. My inclination is to not introduce any restrictions without a strong technical reason to do so.

Do we want to encourage or recommend default export over named exports for routing use cases?

I don't know about encourage. I don't love 3P guidance diverging from Google internal guidance (over which we have limited sway). My inclination is to take no formal stance from a style guide perspective.

Does this fit in ng generate component? Does it fit in ng generate route instead?

I think adding an option is reasonable. I don't love the idea of ng g route, though, since it muddies the water as to types of Angular construct (i.e., I don't want people writing blog posts about "Route components" vs "component components")

angular-robot[bot] commented 1 year ago

This feature request is now candidate for our backlog! In the next phase, the community has 60 days to upvote. If the request receives more than 20 upvotes, we'll move it to our consideration list.

You can find more details about the feature request process in our documentation.

aparzi commented 8 months ago

Hi @alan-agius4, is anyone working on this issue? Otherwise I could do it myself

dgp1130 commented 1 month ago

We had some more discussion today based on recent community interest in this.

We're ok with adding a form of --export-default. I don't think any of us are super excited about the option given that most users are unlikely to discover this or remember to use it and instead just manually add export when needed. We generally like to avoid a large number of small conditionals like --display-block. However there's enough upvotes on this issue to justify at least trying it out. I'd prefer --export-default over --default-export given that aligns with the ordering you'll use when writing export default in actual JavaScript, so it feels easier to remember to me.

Longer term, it might be worth adding some kind of ng generate route which creates a component (with --export-default) and also adds it to the route list. Unfortunately this is tricky to get right as we'd need to know whether to use loadComponent or loadChildren and find the routes.ts file, which could be renamed. That's probably out of scope for now, but it's a potential future feature to consider.

If anyone would like to contribute an --export-default option, it's probably a good first contribution as it's fairly well-scoped to schematics. You'd need to add a new exportDefault option here and update the template accordingly

brandonroberts commented 1 month ago

Awesome. Thanks for the update and consideration!

aparzi commented 4 weeks ago

Hi @dgp1130, I have implemented --export-default, can you help me understand how i can test the command locally?

Thanks.

dgp1130 commented 4 weeks ago

@aparzi, you should definitely add some tests to https://github.com/angular/angular-cli/blob/e40384e637bc6f92c28f4e572f986ca902938ba2/packages/schematics/angular/component/index_spec.ts, which should provide some reasonable confidence.

If you want to test it out in your own application, yarn build --local should build the packages into the dist/ directory. You can then install the @schematics/angular tarball into any Angular app test out ng generate component --export-default directly.

More details: https://github.com/angular/angular-cli/blob/main/docs/DEVELOPER.md

aparzi commented 4 weeks ago

@dgp1130 ok thanks. I open a merge request. I have made the implementation and written some tests

aparzi commented 4 weeks ago

Hi @alan-agius4, should this part of the documentation be updated with the new option?

Thanks

alan-agius4 commented 4 weeks ago

The option will automatically be updated.

See: https://github.com/angular/angular/pull/57508

aparzi commented 4 weeks ago

Awesome