GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — JavaScript LS scaffolding + JS module inference by RyanCavanaugh · Pull Request #5266 · microsoft/TypeScript

Conversation

@RyanCavanaugh

@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) commented Oct 15, 2015

Copy link
Copy Markdown
Member

This is a subset of the prior PR that carves out only the code we need to get a) JavaScript intellisense in general and b) CommonJS module inference in JavaScript files.

Includes work by Vladimir Matveev (@vladima) in 'services' to get the correct compilation context to Roslyn

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@RyanCavanaugh
JavaScript LS scaffolding + JS module inference
1a36fce
@msftclas Microsoft Contribution License Agreements (msftclas) added the cla-not-required label Oct 15, 2015
DanielRosenwasser
DanielRosenwasser reviewed Oct 15, 2015
View reviewed changes
Comment thread src/services/services.ts Outdated

@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) Oct 15, 2015

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Consider extracting this out into a function.

function shouldRecordAmdDefineName(index: number, moduleName: string) {
    // record first item in the list only if its name is not "require"
    // record second item in the list only if its name is not "exports"
    // record third item in the list only if its name is not "module"
    // record all other items in the list unconditionally
    switch (index) {
        case 0:
            return moduleName !== "require";
        case 1:
            return moduleName !== "exports";
    }
    return index !== 2 || moduleName !== "module";
}

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) Oct 15, 2015

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

We can remove the AMD logic entirely

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Ryan Cavanaugh (RyanCavanaugh) added 2 commits October 16, 2015 16:40
@RyanCavanaugh
Address CR feedback
2f7719b
@RyanCavanaugh
Remove obsolute AMD logic from reference preprocessing in services
61b7100
ahejlsberg
ahejlsberg reviewed Oct 19, 2015
View reviewed changes
Comment thread src/compiler/checker.ts Outdated

@ahejlsberg Anders Hejlsberg (ahejlsberg) Oct 19, 2015

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Don't think you need the declaration.parent.kind test. There should never be a case where that isn't true.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Ryan Cavanaugh (RyanCavanaugh) added 3 commits October 19, 2015 11:02
@RyanCavanaugh
CR feedback
5725fc4
@RyanCavanaugh
LKG update
efa63b9
@RyanCavanaugh
Merge remote-tracking branch 'upstream/master' into javaScriptModules
eda6eca
ahejlsberg
ahejlsberg reviewed Oct 22, 2015
View reviewed changes
Comment thread src/compiler/binder.ts Outdated

@ahejlsberg Anders Hejlsberg (ahejlsberg) Oct 22, 2015

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

We usually put a blank on both sides of the | operator.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@RyanCavanaugh

Ryan Cavanaugh (RyanCavanaugh) commented Oct 22, 2015

Copy link
Copy Markdown
Member Author

Vladimir Matveev (@vladima) can you address the comments in program.ts?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Vladimir Matveev (vladima) and others added 8 commits October 22, 2015 14:12
@vladima
do not look into nested es6 exports / imports when collecting externa…
abf270a 
…l modules
@RyanCavanaugh
CR feedback
91eb758
@RyanCavanaugh
Rename isTsx for clarity
7a94031
@RyanCavanaugh
Don't look for .js files when resolving node modules
d880d4f
@RyanCavanaugh
Merge branch 'master' into javaScriptModules
3f4e5a4 
# Conflicts:
#	tests/webTestServer.ts
@RyanCavanaugh
Merge branch 'master' into javaScriptModules
7dd1bf4 
# Conflicts:
#	lib/lib.es6.d.ts
#	lib/tsc.js
#	lib/tsserver.js
#	lib/typescript.d.ts
#	lib/typescript.js
#	lib/typescriptServices.d.ts
#	lib/typescriptServices.js
#	src/compiler/binder.ts
#	src/compiler/checker.ts
#	src/compiler/parser.ts
#	src/compiler/program.ts
#	src/harness/fourslash.ts
@RyanCavanaugh
Fix merge problems from master
e630ce2
@RyanCavanaugh
Merge remote-tracking branch 'origin/master' into javaScriptModules
69ca1f2 
# Conflicts:
#	src/compiler/parser.ts
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@RyanCavanaugh @DanielRosenwasser @ahejlsberg @vladima @msftclas
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.