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

Force Override Declarations Types #36146

Open
2 tasks
dretechtips opened this issue Jan 12, 2020 · 29 comments
Open
2 tasks

Force Override Declarations Types #36146

dretechtips opened this issue Jan 12, 2020 · 29 comments
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@dretechtips
Copy link

dretechtips commented Jan 12, 2020

Search Terms

declaration
module
merging
override
interface

Suggestion

Add a way to force override declarations for modules in the workspace. Typescript allows you to create a global.d.ts to add global declarations. How about an override.d.ts or an *.override.d.ts file to override existing modules declarations in the user workspace.

Use Cases

Lets say you install a node module package and type. You see that the type parameter isn't type safe so you use declaration merging to create a more type safe declaration. However, when you use the module type as a variables type or choose to extending it, it automatically uses the types folder declaration first since the type parameter used fits the node_modules declaration type parameter, however that type would not fit the type parameter the user created. This is because declaration merging selects the most appropriate types in order. If the type was to not match the first declaration type it would move on until reaching the appropriate type, which wouldn't work in this use case.

Examples

// Node Module module declaration file
declare module "react-router" {
  interface RouteComponentProps<Params extends { [K in keyof Params]?: string | undefined }> {
    params: Params;
  }
}

// User Defined Type declaration file
export type NoRequired<T extends {}> = {
  [C in keyof T]: T[C] extends Required<T>[C] ? never : T[C];
};

declare module "react-router" {
  interface RouteComponentProps<Params extends NoRequired<Params> }> {
    params: Params;
  }
}

// Using the Interface
import { RouteComponentProps } from "react-router";

// I don't want to extend RouteComponentProps<{ page: string }>, but I can.
export interface CRUDComponentProps
  extends RouteComponentProps<{ page: string }> {
  serverName: string;
  clientName: string;
}

As you can see the type is accepted as the node modules declaration type instead of a user defined declaration type. There no other way of overriding it other than removing it manually from the the node_modules type file.

Checklist

My suggestion meets these guidelines:

  • A way of force overriding a type (preferably an override.d.ts or an *.override.d.ts file)
  • A way to sort the order in which declaration merging happens
@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Jan 13, 2020

I guess this would work so long as there's only one file like this. I do have some apprehension since users might use this more often in place of actually fixing things on DefinitelyTyped.

@dretechtips
Copy link
Author

dretechtips commented Jan 14, 2020

This could be used as a temporary measure because type definitions have to be reviewed before being updated on DefinitelyTyped. There also no need for one override file, since you can create a user based folder called override and place declaration files with the name of the module your trying to override. Because files can’t have the same name there will be no duplicates of modular overrides.

@RyanCavanaugh RyanCavanaugh added Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript labels Feb 5, 2020
@lixpng
Copy link

lixpng commented Mar 20, 2020

I want this feature too. I use webpack in my project, it has declare the process variable, just like this:

declare global {
    interface __WebpackProcess {
        env: any
    }
    declare var process: __WebpackProcess
}

It declare process.env as any type, but I want change it to a more accurate type, I don't know how to do it.

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented May 20, 2020

This is like the dual of #31894.

@jamescdavis
Copy link

jamescdavis commented Jun 2, 2020

I have a need for this too. For example, I'd like to tighten up the type for QUnit's assert.ok to

ok(state: boolean, message?: string): void;

in a project (but don't think the DT types should be that strict). There's no good way to do this (at least that I'm aware of) short of redefining all of the QUnit types locally and not including @types/qunit.

@samson-sham
Copy link

samson-sham commented Jun 30, 2020

I have a similar need, where I imported a library which is partly typed, and when I do declaration merging/module augmenting for that library, I'm being locked in "declarations must have the same type" issue for days.

@svallory
Copy link

svallory commented Aug 7, 2020

It would be useful also for "special cases" where typings are used to discourage use of API params which are actually valid. E.g. Stipe's create source API method: stripe/stripe-node#974

@daniellwdb
Copy link

daniellwdb commented Aug 20, 2020

Plugins are a good example of a valid use case. fastify has route methods, for example:

get: RouteShorthandMethod<RawServer, RawRequest, RawReply>; but when you create a plugin that changes the behaviour of one of the route methods. you cannot reflect that runtime behaviour in the type system, in this case I'd like to do something like: get: TypedRouteShorthandMethod<RawServer, RawRequest, RawReply>;.

Currently the ugly hack is to automate the process of commenting existing declarations using patch-package (links to real life example of a plugin)

@dbartholomae
Copy link

dbartholomae commented Aug 29, 2020

I'm running into similar problems: I have a library that uses JSX but not React. Since React types are global, when both my library and React are installed, the types clash. I would love to be able to import my types just in the files that use my library without disturbing the typing in other files.

@bengotow
Copy link

bengotow commented Sep 3, 2020

+1! Just wanted to add another use case for this I've encountered:

We use the TypeORM library in a large web application and a few of it's "findBy"-style methods are overly permissive. It's easy to pass arguments which actually have no effect on the generated SQL but seem like they would. We'd like to disallow certain usages as a team by narrowing the type declarations but the package provides it's own types so our only option is to fork it. We'd love to have an overrides file where we could forcibly declare a narrower input type for just a few methods and call it a day!

@svallory
Copy link

svallory commented Sep 19, 2020

@bengotow you could prevent that by writing a custom tslint rule

@Thyiad
Copy link

Thyiad commented Mar 30, 2021

How it's going

@kaangokdemir
Copy link

kaangokdemir commented May 5, 2021

Any progress?

@GiancarlosIO
Copy link

GiancarlosIO commented Jun 1, 2021

it would be great to have because with that we can fix cases like this emotion-js/emotion#1257

@CrackThrough
Copy link

CrackThrough commented Jun 19, 2021

I love this idea. I really hope this gets added in next major update!

@ptim
Copy link

ptim commented Sep 9, 2021

My use case: adding project-specific documentation to the existing types for NextJS.

Currently the ugly hack is to automate the process of commenting existing declarations using patch-package (links to real life example of a plugin)
#issuecomment-677936683

Tx 👍

There is some interesting discussion of further workarounds/hacks over at SO: How to overwrite incorrect TypeScript type definition installed via @types/package.

@anuoua
Copy link

anuoua commented Nov 10, 2021

Any update?

1 similar comment
@DualWield
Copy link

DualWield commented Jan 19, 2022

Any update?

@RyanCavanaugh RyanCavanaugh added In Discussion Not yet reached consensus and removed Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature labels Jan 20, 2022
@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jan 20, 2022

We had some casual conversations about this and are basically terrified of how it would work in practice as soon as more than one person starts using it. What happens when multiple files try to override? What happens when something on DefinitelyTyped claims to override something? What happens if two packages override a global in ways that they both depend on "sticking"? How does a user resolve conflicts? Can you override an entire namespace?

We'll look at it again, though.

@dbartholomae
Copy link

dbartholomae commented Jan 21, 2022

Thanks! It might make sense to look at two solutions for similar problems in the ecosystem:

Based on the discussions in this thread, the first idea might work for TypeScript by having a setting in tsconfig. From my understanding, only one tsconfig file is used at a time, so there would be no way for different settings to contradict each other.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jan 21, 2022

Yeah, I think a tsconfig-based mechanism is the thing that makes the most sense. The problem from there is figuring out the granularity. For "These defs are totally bad and I want to use my own", that's fine. But it'd be cumbersome to say "If you want to fix this one type in this file, you need to make an entire copy of it (and keep it in sync with everything else)".

Then there's the separate question if this is an override of a module name or a file - both have use cases IMHO, since presumably at least some of the time you need to override a global definition.

@dretechtips
Copy link
Author

dretechtips commented Jan 21, 2022

@RyanCavanaugh you don't have to create an entire copy if you use a stacking mechanism to override the files with the greatest number taking precedence. This would be used to override the module. [module_name].override.ts

@rajivharris
Copy link

rajivharris commented Apr 20, 2022

This is a very interesting idea.
This would enable us to add project specific type restrictions to the existing third party types.

@CrackThrough
Copy link

CrackThrough commented Apr 21, 2022

Or just a simple "override" keyword in ts syntax might work as well, I think that's going to make less changes.

@antoniopresto
Copy link

antoniopresto commented Jul 21, 2022

This just worked 🎉

// override.d.ts (note the d.ts ⚠️)

declare global {
  module '@darch/schema' {
    export * from '@darch/schema';  // 👈🏼 export the same module

    // Function override
    export function createType(def:{
      union: [
        'string',
        'int',
        '[float]', //
      ],
    }): "WHATEVER2";
  }
}

Kapture 2022-07-20 at 22 06 26

@dac09
Copy link

dac09 commented Jul 21, 2022

@antoniopresto - interesting solution - hadn't thought of this before!

I get the error "Ambient modules cannot be nested in other modules or namespaces.ts(2435)" when I wrap my module in declare global.

Screenshot 2022-07-21 at 15 57 03

If I try to override the module just by declare module, I'm seeing this error in on the export line: "Exports and export assignments are not permitted in module augmentations.ts(2666)"

Screenshot 2022-07-21 at 15 55 47

I wonder if there's something setting your project that's letting you do this? Does @darch/schema exist as a module in your project?


I'm really hoping for something like an override keyword, as mentioned in a comment above.

@antoniopresto
Copy link

antoniopresto commented Jul 21, 2022

@dac09 @darch/schema is installed.
this is my tsconfig, nothing special, I think.

{
  "ts-node": {
    "transpileOnly": true, 
    "compilerOptions": {
      "module": "commonjs"
  },
  "compilerOptions": {
    "outDir": "dist",
    "rootDir": "src",
    "baseUrl": "./src",
    "paths": {
      "@utils/*": ["utils/*"],
      "@core/*": ["core/*"],
      "@services/*": ["services/*"],
      "@appTypes/*": ["types/*"],
      "@entity/*": ["entity/*"]
    },
    "lib": ["es5", "es6", "es7", "esnext", "dom", "es2020"],
    "declaration": true,
    "target": "es5",
    "module": "esnext",
    "removeComments": false,
    "esModuleInterop": true,
    "moduleResolution": "node",
    "resolveJsonModule": true,
    "strict": true,
    "skipLibCheck": true,
    "strictPropertyInitialization": false,
    "noUnusedLocals": true,
    "noUnusedParameters": true,
    "noImplicitReturns": true,
    "noFallthroughCasesInSwitch": true,
    "downlevelIteration": true,
    "noImplicitAny": false
  },
  "include": ["./src/**/*"]
}

@antoniopresto
Copy link

antoniopresto commented Jul 21, 2022

@dac09 are you also using a *.d.ts file?
I tried ts 4.5.4 and the latest 4.7.4 - both worked

@dac09
Copy link

dac09 commented Jul 26, 2022

@dac09 are you also using a *.d.ts file?
I tried ts 4.5.4 and the latest 4.7.4 - both worked

Hi Antonio, yes I am. The file is an ambient declaration that currently “merges” the interfaces for “CurrentUser” (and works) - so don’t see any reason why it’s different. Anyone else have any thoughts what the difference might be? I have a feeling it’s the “skipLibCheck” flag being enabled in your project - the implications of which I’m unsure of.

You can actually see this behaviour in any RedwoodJS project, where the ambient declarations sit in “.redwood/types”, after type generation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests