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.

Sign up for GitHub

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

Jump to bottom

Adding different module resolution strategies #4352

Merged
merged 8 commits into from
Aug 24, 2015
Merged

Adding different module resolution strategies #4352

merged 8 commits into from
Aug 24, 2015

Conversation

vladima
Copy link
Contributor

@vladima vladima commented Aug 18, 2015

No description provided.

mhegazy
mhegazy reviewed Aug 18, 2015
View reviewed changes
src/compiler/program.ts
}

// module name has one of supported extesions
for(let ext of supportedExtensions ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I do not think we need this since SystemJS allowes it

Copy link
Member

Choose a reason for hiding this comment

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

Space after for

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

@vilicvane
Copy link

!!! Fantastic, finally got here. Thanks!

yuit
yuit reviewed Aug 19, 2015
View reviewed changes
src/compiler/types.ts


export const enum ModuleResolutionKind {
Legacy = 1,
Copy link
Contributor

Choose a reason for hiding this comment

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

What is Legacy? Could you add comment?

@alexeagle
Copy link
Contributor

+cc @mprobst

mhegazy
mhegazy reviewed Aug 20, 2015
View reviewed changes
src/harness/harness.ts
@@ -1025,6 +1025,9 @@ module Harness {
options.module = ts.ModuleKind.UMD;
} else if (setting.value.toLowerCase() === "commonjs") {
options.module = ts.ModuleKind.CommonJS;
if (options.moduleResolution === undefined) {
options.moduleResolution = ts.ModuleResolutionKind.Classic;
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add a comment

@mhegazy
Copy link
Contributor

mhegazy commented Aug 20, 2015

👍

mhegazy added a commit that referenced this pull request Aug 24, 2015
@mhegazy
Merge pull request #4352 from Microsoft/moduleResolutionStrategies
9a6db93 
Adding different module resolution strategies
@mhegazy mhegazy merged commit 9a6db93 into master Aug 24, 2015
@mhegazy mhegazy deleted the moduleResolutionStrategies branch August 24, 2015 23:03
@mihailik
Copy link
Contributor

Is there a description of what this is doing and how to use it please?

@vilicvane
Copy link

@mihailik You may specify "moduleResolution" in compilerOptions as "node", then it will try to load declaration file under related node module. For me it seems that either index.d.ts or specified "typings" field in package.json works, not typings related to "main" field.

@mhegazy
Copy link
Contributor

mhegazy commented Aug 25, 2015

one thing to add, --m commonjs means your moduleResolution is node, so if this is what you are using, you get the new behavior automatically.

let us know if you run into issues with the new resolution logic.

@jbrantly jbrantly mentioned this pull request Aug 25, 2015
Closed
This was referenced Aug 31, 2015
Closed
Closed
@jbrantly jbrantly mentioned this pull request Sep 1, 2015
Closed
@tetsuharuohzeki tetsuharuohzeki mentioned this pull request Sep 1, 2015
Merged
@mhegazy mhegazy mentioned this pull request Sep 5, 2015
Closed
@mhegazy mhegazy mentioned this pull request Sep 15, 2015
Closed
@weswigham weswigham mentioned this pull request Oct 14, 2015
Merged
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@vladima @vilicvane @alexeagle @mhegazy @mihailik @yuit @DanielRosenwasser @msftclas