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

Why is Route#data a type instead of an interface? #38111

Open
nartc opened this issue Jul 17, 2020 · 3 comments
Open

Why is Route#data a type instead of an interface? #38111

nartc opened this issue Jul 17, 2020 · 3 comments
Labels
area: router cross-cutting: types feature: under consideration Feature request for which voting has completed and the request is now under consideration feature: votes required Feature request which is currently still in the voting phase feature Issue that requests a new feature freq4: critical P4 A relatively minor issue that is not relevant to core functions state: confirmed
Milestone

Comments

@nartc
Copy link
Contributor

nartc commented Jul 17, 2020

🐞 bug report

Affected Package

The issue is caused by package @angular/router

Description

A clear and concise description of the problem...

When we work on the routing configuration, sometimes we leverage the Route#data property to add metadata for a specific route. Eg: permissions information like the following:

image

In my use-case, I'd like to type Route#data to have an optional property requiredPermission which is a Tuple of [PermissionNames, Privilege]. And I want to enforce this type specifically for requiredPermission if it is added to a Route#data. Normally, I'd just go to typings.d.ts (or whatever *.d.ts you have at root) and override a library's interface there. Eg:

image

But for the case of Route#data, it has type Data = {} tied to it and Type does not allow for Declaration Merging so TypeScript doesn't allow me to override Route#data as the above screenshot shows. This prevents me from enforcing the correct typings for requiredPermission, hence leads to bad Developer Experience (in our cases) and Runtime bugs (instead of Compilation time).

If Route#data was an interface (which allows for Declaration Merging), I could indeed achieve the following:
image
image
image

Before writing up this issue, I was going to submit a PR right away to "fix" the bug. But looking at the source code, there are many different places (other than Route#data) being given types with Type instead of interface.

  • Is there a reason for using Type rather than Interface for these cases?
  • If I were to submit a PR, would I need to take into consideration of other Type other than Route#data to change those into Interface?

🔬 Minimal Reproduction

A good way to reproduce this (as the screenshots above have shown) is to:

  1. Create a new Angular app (or use existing Angular app)
  2. Create a typings.d.ts on the same level as main.ts (or use existing *.d.ts on that same level)
  3. Try to merge Data to have custom typed properties:
declare module '@angular/router'  {
   interface Data {
      requiredPermission?: [number, string]
   }
}
  1. TypeScript will show error: Duplicate identifier Data

Issues that don't have enough info and can't be reproduced will be closed.

You can read more about issue submission guidelines here: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#-submitting-an-issue
-->

🌍 Your Environment

Angular Version:





     _                      _                 ____ _     ___
    / \   _ __   __ _ _   _| | __ _ _ __     / ___| |   |_ _|
   / △ \ | '_ \ / _` | | | | |/ _` | '__|   | |   | |    | |
  / ___ \| | | | (_| | |_| | | (_| | |      | |___| |___ | |
 /_/   \_\_| |_|\__, |\__,_|_|\__,_|_|       \____|_____|___|
                |___/


Angular CLI: 9.1.8
Node: 12.13.0
OS: darwin x64

Angular:
...
Ivy Workspace:

Package                      Version
------------------------------------------------------
@angular-devkit/architect    0.901.8
@angular-devkit/core         9.1.8
@angular-devkit/schematics   9.1.8
@schematics/angular          9.1.8
@schematics/update           0.901.8
rxjs                         6.5.4
@nartc nartc changed the title Why Route#data is a type instead of an interface? Why is Route#data a type instead of an interface? Jul 17, 2020
@ngbot ngbot bot added this to the needsTriage milestone Jul 17, 2020
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Jul 17, 2020
@atscott
Copy link
Contributor

atscott commented Jul 17, 2020

Related issue #27240

@jelbourn jelbourn added P4 A relatively minor issue that is not relevant to core functions and removed severity2: inconvenient labels Oct 1, 2020
@angular-robot angular-robot bot added the feature: votes required Feature request which is currently still in the voting phase label Jun 4, 2021
@angular-robot
Copy link
Contributor

angular-robot bot commented Jun 4, 2021

Just a heads up that we kicked off a community voting process for your feature request. There are 20 days until the voting process ends.

Find more details about Angular's feature request process in our documentation.

@angular-robot angular-robot bot added the feature: under consideration Feature request for which voting has completed and the request is now under consideration label Jun 18, 2021
@Erbenos
Copy link
Contributor

Erbenos commented Jan 25, 2024

Hypothetically you can already extend Route interface itself rather than the Route.data. Given that Route.data is written to by eg.: route resolvers, perhaps that may already be preferential?

Conflicts can be worked around by using Symbols, except that you would lose autocomplete see microsoft/TypeScript#54870, grouping all metadata under single key, and/or using some property name prefixing I suppose.

Any such extension would be accessible via eg.: ActivatedRoute.routeConfig.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: router cross-cutting: types feature: under consideration Feature request for which voting has completed and the request is now under consideration feature: votes required Feature request which is currently still in the voting phase feature Issue that requests a new feature freq4: critical P4 A relatively minor issue that is not relevant to core functions state: confirmed
Projects
None yet
Development

No branches or pull requests

5 participants