Skip to content

Check if imported file is a proper external module for imported node typings - #4738

Merged
Vladimir Matveev (vladima) merged 4 commits into
masterfrom
properExternalModules
Sep 11, 2015
Merged

Check if imported file is a proper external module for imported node typings#4738
Vladimir Matveev (vladima) merged 4 commits into
masterfrom
properExternalModules

Conversation

@vladima

@vladima Vladimir Matveev (vladima) commented Sep 10, 2015

Copy link
Copy Markdown
Contributor

No description provided.

mhegazy
mhegazy reviewed Sep 10, 2015
View reviewed changes
Comment thread src/compiler/types.ts Outdated

@mhegazy Mohamed Hegazy (mhegazy) Sep 10, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i think this should be declarative. i.e. the name tells you what it is rather than telling you how it should be have. so something like
isExternalLibrary, or isExternalLibraryImport

@mhegazy

Mohamed Hegazy (mhegazy) commented Sep 10, 2015

Copy link
Copy Markdown
Contributor

also looks like you have build error.

@vladima Vladimir Matveev (vladima) mentioned this pull request Sep 11, 2015
Merged
@mhegazy

Mohamed Hegazy (mhegazy) commented Sep 11, 2015

Copy link
Copy Markdown
Contributor

👍

Vladimir Matveev (vladima) added a commit that referenced this pull request Sep 11, 2015
@vladima
Merge pull request #4738 from Microsoft/properExternalModules
026632b 
Check if imported file is a proper external module for imported node typings
@vladima
Vladimir Matveev (vladima) merged commit 026632b into master Sep 11, 2015
@vladima
Vladimir Matveev (vladima) deleted the properExternalModules branch September 11, 2015 17:20
@vladima

Vladimir Matveev (vladima) commented Sep 12, 2015

Copy link
Copy Markdown
Contributor Author

pinging Basarat Ali Syed (@basarat) and James Brantly (@jbrantly): FYI, this PR has changed the signature of CompilerHost.resolveModuleNames to return ResolvedModule instead of string

@jbrantly

James Brantly (jbrantly) commented Sep 12, 2015

Copy link
Copy Markdown

Thanks for the heads up

@mhegazy Mohamed Hegazy (mhegazy) added the Breaking Change Would introduce errors in existing code label Sep 12, 2015
@mhegazy

Mohamed Hegazy (mhegazy) commented Sep 12, 2015

Copy link
Copy Markdown
Contributor

Vladimir Matveev (@vladima) can you comment this issue in the API breaking changes page

jbrantly
jbrantly reviewed Sep 12, 2015
View reviewed changes
Comment thread tests/baselines/reference/nodeResolution7.errors.txt

@jbrantly James Brantly (jbrantly) Sep 12, 2015

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It seems like this would be a good case of having the "Please contact the package author to update the package definition." message.

@vladima Vladimir Matveev (vladima) Sep 12, 2015

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fair point, will update the message. Thanks!

James Brantly (jbrantly) added a commit to TypeStrong/ts-loader that referenced this pull request Sep 12, 2015
@jbrantly
Update custom resolution logic to work with latest API changes (see m…
7b81715 
…icrosoft/TypeScript#4738)
Stanislav Panferov (s-panferov) added a commit to s-panferov/awesome-typescript-loader that referenced this pull request Sep 13, 2015
@s-panferov
feat(*): update module resolution logic according to microsoft/TypeSc…
27d799c 
…ript#4738
@jbrantly James Brantly (jbrantly) mentioned this pull request Sep 15, 2015
Closed
Stanislav Panferov (s-panferov) added a commit to s-panferov/awesome-typescript-loader that referenced this pull request Sep 17, 2015
@s-panferov Stanislav Panferov
feat(*): update module resolution logic according to microsoft/TypeSc…
59ff7f7 
…ript#4738
@mhegazy Mohamed Hegazy (mhegazy) mentioned this pull request Sep 17, 2015
Closed
@weswigham Wesley Wigham (weswigham) mentioned this pull request Sep 18, 2015
Closed
@jbrantly James Brantly (jbrantly) mentioned this pull request Sep 22, 2015
Closed
@microsoft 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

Breaking Change Would introduce errors in existing code

Projects

None yet

Milestone

TypeScript 1.6.2

Development

Successfully merging this pull request may close these issues.

4 participants

@vladima @mhegazy @jbrantly @msftclas