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

feat: improve declaration merging resolution #33327

Closed
4 tasks done
jorroll opened this issue Sep 9, 2019 · 3 comments
Closed
4 tasks done

feat: improve declaration merging resolution #33327

jorroll opened this issue Sep 9, 2019 · 3 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@jorroll
Copy link

jorroll commented Sep 9, 2019

Search Terms

declaration merging imported interface

Suggestion

this is an extension of #33326 and this S.O. issue

At the moment, declaration merging requires you to specify the original file that an interface was declared in.

For example, given a module @rschedule/core with the following files

// @rschedule/core/es2015/packages/core/core/date-adapter

export interface DateAdapterType {
  base: DateAdapterBase;
}
// @rschedule/core/index.ts

export * from './es2015/packages/core/core/date-adapter'

You cannot currently perform declaration merging using the following

// file_a.ts

declare module '@rschedule/core' {
  interface DateAdapterType {
    one: string;
  }
}

The problem appears to be that @rschedule/core is re-exporting DateAdapterType. Instead you must do

// file_a.ts

declare module '@rschedule/core/es2015/packages/core/core/date-adapter' {
  interface DateAdapterType {
    one: string;
  }
}

This is problematic because the original location of the DateAdapterType is private to the @rschedule/core module.

This is a feature request to let developers use declaration merging via

// file_a.ts

declare module '@rschedule/core' {
  interface DateAdapterType {
    one: string;
  }
}

Use Cases

The ability to better encapsulate modules and only exposing interfaces via a single point. The current API seems to require that module type declarations are flattened in some way and declared directly in @rschedule/core/index.d.ts. This necessitates a special build setup. I'm not immediately clear on how I can satisfy the current API, practically speaking.

Developer happiness. I would assert that this feature request would make typescript more intuitive, as the current design limitation requires knowledge of the typescript implementation (i.e. the current design doesn't seem to make any logical sense, other than, perhaps (?), for performance reasons).

Examples

shown above

Checklist

My suggestion meets these guidelines:

  • [?] This wouldn't be a breaking change in existing TypeScript/JavaScript code

    • I'm not sure if this would be a breaking change. Is it possible that a module is relying on the current behavior? I don't think so, but maybe someone from the community can pipe up and say otherwise. I think this is a non-breaking change though, as I view the current behavior as a bug.
  • This wouldn't change the runtime behavior of existing JavaScript code

  • This could be implemented without emitting different JS based on the types of the expressions

  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)

  • This feature would agree with the rest of TypeScript's Design Goals.

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Sep 13, 2019
@RyanCavanaugh
Copy link
Member

We can't do this because it would change the semantics of existing programs in very problematic ways. Declaration merging with modules is difficult and I think there's room for improvement in terms of making it easier to augment existing modules, but this isn't a possible path forward.

@jorroll
Copy link
Author

jorroll commented Sep 13, 2019

I'd be happy with anything that addresses the spirit of the proposal / improves the ergonomics of declaration merging with modules.

Simply improving the documentation in this area (#33326) would be a big help though.

@typescript-bot
Copy link
Collaborator

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

3 participants