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

Allow relative module IDs in ambient external module declarations? #1720

Closed
csnover opened this issue Jan 19, 2015 · 8 comments
Closed

Allow relative module IDs in ambient external module declarations? #1720

csnover opened this issue Jan 19, 2015 · 8 comments
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript

Comments

@csnover
Copy link
Contributor

csnover commented Jan 19, 2015

Per direction from @mhegazy I’m opening this ticket as a discussion point for loosening the restriction forbidding relative module IDs in ImportDeclarations in AmbientModuleDeclaration. Loosening the restriction prevents the need to rewrite import declarations when combining multiple external declarations to a single file, and maybe simplifies some internals of the compiler (I haven’t checked, but it should eliminate some code branches).

Relative module IDs should be resolvable by the same way that the rest of the tsc machinery works, by resolving the reference according to the StringLiteral in a module’s own AmbientExternalModuleDeclaration. For example:

declare module 'foo/Foo' {
  import Bar = require('./Bar'); // resolve to `foo/Bar`
}

Right now you are not allowed to do this and need to write:

declare module 'foo/Foo' {
  import Bar = require('foo/Bar');
}
@danquirk danquirk added the Suggestion An idea for TypeScript label Jan 19, 2015
@mhegazy
Copy link
Contributor

mhegazy commented Dec 8, 2015

The main issue here is that semantics and resolution logic of relative module names in ambient context are confusing/unclear. looks like the correct solution is something along the lines of #4433

@mhegazy mhegazy closed this as completed Dec 8, 2015
@mhegazy mhegazy added the Declined The issue was declined as something which matches the TypeScript vision label Dec 8, 2015
@csnover
Copy link
Contributor Author

csnover commented Dec 9, 2015

Could you maybe give a little more colour about what is confusing/unclear about relative module name resolution in this context?

@mhegazy
Copy link
Contributor

mhegazy commented Dec 9, 2015

when resolving a relative module import in an external module (a file that has at least one top level import or export statement/declaration), it is always assumed that the module name to be relative to the current file, this works as module names are their containing file path. For ambient module declarations (i,e, something of the form declare module ".\mod", what are they relative to, is it the file where they are defined (i.e. the .d.ts)? or is it relative to the file where they are imported? should we look for files with the same name as well? what about the new basepath support, do we go look for them in the basepath as well? is that dependent on the project? or should they always be resolved the same way?

the issue that motivated the OP, if i am not wrong, is the need to collect a set of ambient external modules, and generate a single .d.ts file from them. #5090 achieves something like this today, by making all module names non-relative based on the project root directory.

@csnover
Copy link
Contributor Author

csnover commented Dec 9, 2015

This ticket is specifically about the import statements, not declare module statements. There is no ambiguity when you are in module 'a/b' and you import './c', your import is 'a/c'. The only time there is ambiguity/failure is when you try to ascend above the top of the module ID hierarchy.

The rationale for this ticket originally was that it makes it easier to bundle modules into a single d.ts because it doesn’t require rewriting of the import statements to make them absolute module IDs. This would make a d.ts generator very simple since all it would need to do is read in each TS module, get the declarations out of the compiler, and wrap with declare module 'foo' { …code from compiler… }. Currently you also need to rewrite all the imports to make them absolute.

Finally, with regards to relative declare module statements, I have a very simple and intuitive answer, based on why you would ever actually do something like that: such IDs would only be valid in a typings file referenced by package.json, and would be relative to the package’s root. This then allows packages to be written with typings that are fully portable (i.e. can be safely relocated, critical for whenever you have two competing versions of a single package, which is very common). But, that isn’t really about this ticket anyway.

@mightyiam
Copy link

I have a Webpack/typescript setup where some of the imports are not files, but are autogenerated by a Webpack 'loader'. So, for typescript to be happy about the fact that these files do not exist at all, I'd like to be able to work freely with ambient declaration files:

import { Metadata } from './interfaces'
declare module './metadata-loader!' {
  const metadata: Metadata
  export default metadata
}

As I see it, paths should be relative to this declaration file. Is this only my narrow point of view?

This will make working with Webpack loaders straightforward, the way I see it.

@mightyiam
Copy link

This also does not seem to work:

import { Metadata } from './interfaces'
declare module '*metadata-loader!' { // notice wildcard
  const metadata: Metadata
  export default metadata
}

It fails with

TS2307: Cannot find module './metadata-loader!'.

Which is rather unhelpful. Shouldn't it be an error with the declaration file?

@mhegazy
Copy link
Contributor

mhegazy commented Apr 16, 2017

A declare module ".." block inside another module is an "argumentation" and not a declaration; a previous declaration of this module needs to exist in the global scope to be augmentable.

I am guessing you do not want to augment, but rather declare module '*metadata-loader!'. in this case, just add the declaration to the global scope by adding it to a .d.ts file and including it in your project.

@bennypowers
Copy link

Howdy. I'd like to create d.ts files for my app's javascript modules.

To that end, I'd like to do something like this silly example

// app/index.d.ts
declare module "./app" { 
  class App {
    method(params: Params): Boolean
}
// app/app.js
class App {
  method(...args) { return args.reduce((a, c) => a + c, 0) > 1 }
}

However, I receive the error

[ts] Invalid module name in augmentation. Module './app' resolves to an untyped module at '//path/to/app/app.js', which cannot be augmented.

I'd like to continue to write my project in JavaScript, but leverage TS' excellent editor tooling. Is there some other way to implement this?

Thank you for considering my case.

@microsoft microsoft locked and limited conversation to collaborators Jul 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants