Skip to content

Path mapping module resolution #5728

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

Merged
merged 33 commits into from
Jan 27, 2016
Merged

Path mapping module resolution #5728

merged 33 commits into from
Jan 27, 2016

Conversation

vladima
Copy link
Contributor

@vladima vladima commented Nov 20, 2015

initial implementation for this proposal: #5039.

  • - verify that this module resolution strategy works in VSCode
  • - this strategy does not work with Visual Studio as is but the reason is on the managed side (fix is already submitted). I've verified that once this fix is applied it starts working with with Visual Studio without any changes on the script side (thus check here)

@lucamorelli
Copy link

I suppose this can solve problems I had using angular2 with jspm with Visual Studio. How can try if this works?

@dsebastien
Copy link

Great, I'll try this out as soon as it is merged :)

// @file: /a/b/c/generated/form1.content.ts
// export var x = ...
// first './form.content.ts' needs to be converted to non-relative name.
// 1. convert it to absolute name '/a/b/c/src/form1.content.ts' ->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

form1.content.ts is form.content.ts

@yuit
Copy link
Contributor

yuit commented Nov 24, 2015

can you add test with suffix in paths matching like

"paths": {
     "generated/*/folder1" : ["build/folder1"]

as well as the case when there are multiple match but we pick the longest one (so we have a record of what we use to do in case of future change)

@yuit
Copy link
Contributor

yuit commented Nov 24, 2015

Also what happens in this case:

    "rootDirs": [
        "*",
        "generated/*" 
    ],

is this allowed? if so could we have some tests as well? (from the spec, it seems so. I can't tell from the comment though)
also what happen:

    "rootDirs": [
        "*",
        "generated/*/folder1" 
    ],

@vladima
Copy link
Contributor Author

vladima commented Nov 24, 2015

@yuit

  • sure, I can add test for path mappings where '*' appear in the middle of the substitution string
  • "' * ' in rootDirs" this is part that slightly diverges from the spec (I have not updated it yet): since rootDirs are always prefixes, the only shape where '*' makes sense is <prefix> *. To avoid confusion now rootDirs are just strings.

@alexeagle
Copy link
Contributor

Can you enable the --traceModuleResolution option when running through the language services?

@vladima
Copy link
Contributor Author

vladima commented Jan 13, 2016

Sure, I'll add it in the next iteration of this PR

@prabirshrestha
Copy link
Member

Here is my folder structure

├───app
│       app.ts
│       tsconfig.json
│       webpack.config.js
│
└───libs
    └───guid
        │   package.json
        │
        ├───dist
        │       guid.d.ts
        │       guid.js
        │       guid.js.map
        │
        └───src
                guid.ts
                tsconfig.json

libs/guid/package.json:

{
  "name": "guid",
  "version": "1.0.0",
  "main": "dist/guid.js",
  "typings": "dist/guid.d.ts"
}

app/app.ts:

import guid from 'libs/guid';

console.log(guid());

app/tsconfig.json

{
    "compilerOptions": {
        "declaration": true,
        "module": "commonjs",
        "noImplicitAny": true,
        "removeComments": false,
        "sourceMap": true,
        "outDir": "dist/",
        "target": "es5",
        "moduleResolution": "baseUrl",
        "baseUrl": "",
        "paths": {
            "libs/guid": ["../libs/guid/dist/guid.d.ts"]
        }
    },
    "exclude": [
        "dist"
    ]
}

Rather than specifying paths as "libs/guid": ["../libs/guid/dist/guid.d.ts"] would it be possible to only specify "libs/guid": ["../libs/guid"] and if package.json with typings exists refer to it? This way the libs/guid/package.json controls the entry point rather than caller.

@prabirshrestha
Copy link
Member

Also how do I set path mappings in csproj?

@vladima
Copy link
Contributor Author

vladima commented Jan 21, 2016

I'm currently making a few adjustments to this PR based on results of the latest design meeting and I think that your initial scenario should be supported. Regarding specifying path mappings in .csproj - this won't work, paths and rootDirs will be options that can be specified only via tsconfig.json

trace(host, Diagnostics.Module_name_0_matched_pattern_1, moduleName, matchedPattern);
}

for (const subst of compilerOptions.paths[matchedPattern]) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In your proposal example, it shows paths can be defined as the follows:

"pattern-1": "substitution" | ["list of substitutions"]

However, when const subst of compilerOptions.paths[matchedPattern] is ran, if my path's substitution is a string, of will iterate over each character of the string. Does this need to change or perhaps normalize the compiler options?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the proposal needs to be adjusted - it should always be the list

@prabirshrestha
Copy link
Member

@vladima Would really love to see it working in csproj since that is what we are using internally.
Or is there any way to tell csproj to use tsconfig.json?

I was expecting something like this in csproj.

<TypeScriptPath>
  <Pattern>libs/guid<Pattern>
  <Substitution>../libs/guid</Substitution>
<TypeScriptPath>

{
// this option can only be specified in tsconfig.json
// use type = object to copy the value as-is
name: "rootDirs",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leave a line note

},
description: Diagnostics.Specifies_module_resolution_strategy_Colon_node_Node_js_or_classic_TypeScript_pre_1_6,
error: Diagnostics.Argument_for_moduleResolution_option_must_be_node_or_classic,
error: Diagnostics.Argument_for_moduleResolution_option_must_be_node_classic_or_baseUrl,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove "baseURL" from the message

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@mhegazy
Copy link
Contributor

mhegazy commented Jan 27, 2016

👍

@gsaandy
Copy link

gsaandy commented Mar 27, 2016

which version of typescript and gulp-typescript support this?

@mhegazy
Copy link
Contributor

mhegazy commented Mar 28, 2016

which version of typescript

TypeScript@next and should be in the next release TS 2.0

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.