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

Support consistent module type augmentation #6839

Closed
Cretezy opened this issue Jul 22, 2020 · 11 comments
Closed

Support consistent module type augmentation #6839

Cretezy opened this issue Jul 22, 2020 · 11 comments
Labels

Comments

@Cretezy
Copy link

Cretezy commented Jul 22, 2020

Currently running into some issues while trying to do type augmentation on a third-party module.

Context

Let's say we have module X which defines a base type, in this case ICtxData. We have module Y which augments type, and our application which uses both.

If the application uses the same import for X that module Y uses to augment, the augmentation is working:

// X
interface ICtxData {}

// Y
declare module "https://raw.githubusercontent.com/routexjs/deroutex/master/mod.ts" {
    interface ICtxData {
        body?: any;
        bodyParser?: string
    }
}

// Application
import "y"; // mock
import "https://raw.githubusercontent.com/routexjs/deroutex/master/mod.ts";

// string | undefined
ctx.data.bodyParser

However, if it uses a different import, it does not work:

// X
interface ICtxData {}

// Y
declare module "https://raw.githubusercontent.com/routexjs/deroutex/master/mod.ts" {
    interface ICtxData {
        body?: any;
        bodyParser?: string
    }
}

// Application
import "y"; // mock
import "https://raw.githubusercontent.com/routexjs/deroutex/45caec11f9892ba8137bffd9ac97f9765895c743/mod.ts";

// any
ctx.data.bodyParser

Solution

I'm not sure how Deno could solve this, since we don't have identifiers for module.

@kitsonk
Copy link
Contributor

kitsonk commented Jul 22, 2020

As you pointed out Deno does support it. You just have to know the module name. Let alone augmenting existing TypeScript is "dangerous" in my opinion, like augmenting built in globals.

Your best approach is to import whatever you want, augment it, and re-export it, and any code the needs the argumentation would depend on that re-export.

@brainkim
Copy link

Hi, I’m running into module augmentation issues as well. I’m trying to import a JS file with a typescript generated d.ts reference via unpkg, but the file contains a module augmentation error.

server.tsx

/** @jsx createElement */
// @deno-types="https://unpkg.com/@bikeshaving/crank@0.3.0/index.d.ts"
import {createElement} from "https://unpkg.com/@bikeshaving/crank@0.3.0/index.js";
// @deno-types="https://unpkg.com/@bikeshaving/crank@0.3.0/html.d.ts"
import {renderer} from "https://unpkg.com/@bikeshaving/crank@0.3.0/html.js";


console.log(renderer.render(<div>Hello world</div>, document.body));

tsconfig.json

{
  "compilerOptions": {
    "target": "esnext",
    "module": "esnext",
    "lib": ["esnext", "dom"]
  }
}

command

deno run -c tsconfig.json server.tsx

The above code throws the following error

[ERROR]: Invalid module name in augmentation. Module './index' resolves to an untyped module at 'https://unpkg.com/@bikeshaving/crank@0.3.0/index.js', which cannot be augmented.
declare module "./index" {

because of this code in https://unpkg.com/@bikeshaving/crank@0.3.0/html.d.ts.

declare module "./index" {
    interface EventMap extends GlobalEventHandlersEventMap {
    }
}

Let alone augmenting existing TypeScript is "dangerous" in my opinion, like augmenting built in globals.

Augmenting code is a useful pattern when you don’t want plugins and extensions to clash, I’d hate to see “best practices” stuff block module augmentation support in Deno. I’m thinking of creating an artificial global module to solve this issue, but it would be nice for relative path module augmentation to work too.

@Cretezy
Copy link
Author

Cretezy commented Jul 31, 2020

I think the main "issue" here is the non-static module names (modules can be imported from multiple URLs). I'm going to leave this open, but I too would consider this a non-issue, and should be done in a more proper way if possible.

@Skillz4Killz
Copy link

@Cretezy A simple solution is to create a tsconfig.json file where the includes: [] has the files that declare these augmentations.

@Cretezy
Copy link
Author

Cretezy commented Sep 26, 2020

@Skillz4Killz That is not the problem. The problem is the module name can change based on the import path (re-read the issue please!)

@Cretezy Cretezy changed the title Support module type augmentation Support consistent module type augmentation Sep 26, 2020
@Skillz4Killz
Copy link

Skillz4Killz commented Sep 26, 2020

@Cretezy Perhaps I am seriously misunderstanding your issue, but I believe I had the same issue on my project. All I had to do was move imports to deps.ts file and augment the deps.ts file itself.

declare module "../../deps.ts" {
  interface Member {
    id: string;
    joinedAt: number;
    tag: string;
    avatarURL: string;
    mention: string;
  }
}

All you need to do is import from a common source that you edit, such as deps.ts

@Cretezy
Copy link
Author

Cretezy commented Sep 26, 2020

@Skillz4Killz This is more for augmenting 3rd-party packages. For example:

  • App imports package A and B
  • Package A depends on package B, and augments it
    • Package A imports package B using URL B1
  • App imports package B using URL B2

In this case, package A can't augment package B, because the import tree is different:

--App
---- Package A
------ Package B (B1)
---- Package B (B2)

@stale
Copy link

stale bot commented Jan 6, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jan 6, 2021
@stale stale bot closed this as completed Jan 13, 2021
@Skillz4Killz
Copy link

Not stale, still very much an issue that needs a solution.

@lucacasonato lucacasonato reopened this Jan 13, 2021
@stale stale bot removed the stale label Jan 13, 2021
@stale
Copy link

stale bot commented Mar 14, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Mar 14, 2021
@kitsonk
Copy link
Contributor

kitsonk commented Mar 14, 2021

There is no logical action we can take on this.

If you want a consistent module name, consider using import-maps.json. Also, TypeScript supports wildcards in module specifiers.

@kitsonk kitsonk closed this as completed Mar 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants