Skip to content

Create Wrapper Script for Installing Typings - #1143

Merged
Sara Itani (mousetraps) merged 11 commits into
microsoft:masterfrom
mjbvz:typings-wrapper
Jul 20, 2016
Merged

Create Wrapper Script for Installing Typings#1143
Sara Itani (mousetraps) merged 11 commits into
microsoft:masterfrom
mjbvz:typings-wrapper

Conversation

@mjbvz

@mjbvz Matt Bierner (mjbvz) commented Jul 15, 2016

Copy link
Copy Markdown

Issue #1140

Bug

If a single package in the current project does not have a typings file, we fail to install any typings. This results in a poor IntelliSense experience.

Fix

Create a node script that wraps typings-core. This script will continue installation of all packages, even if some packages do not have typings files.

Testing

Tested on Node 6 so far with a few different scenarios, including projects where the typings for an installed package do not exist. Still have to test on earlier versions of node.

Closes #1140

**Bug**

**Fix**
Create a node script that wraps `typings-core`. This script will continue installation of all packages, even if some packages do not have typings files.

Closes microsoft#1140
@@ -0,0 +1 @@

No newline at end of file

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Seems that npm requires some sort of index/main file, so I just created this empty one.

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.

As discussed, let's move the core logic into index.js

@mjbvz

Copy link
Copy Markdown
Author

Did a basic test pass with Node 0.12. My main concern was whether local module installs would behave, but it seems to work just fine.

Comment thread .gitignore
@@ -1,5 +1,4 @@
# Build Folders (you can keep bin if you'd like, to store dlls and pdbs)
[Bb]in/

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

These were ignoring the bin directory in the new local npm module. I thought it was better to remove them rather than override include the bin directory. No other new files seem to have been added from this

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 would prefer not to include bin by default, as this will likely be an issue if certain projects are built that aren't hooked up into our default project targets

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'll just exclude top level /bin dirs then? Or revert the gitignore change and force add the node bin files?

"author": "Microsoft",
"version": "1.0.0",
"description": "Internal tool that aquires typings to power Node.js Tools For Visual Studio IntelliSense",
"license": "Apache 2",

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.

Apache 2.0

@mousetraps

Copy link
Copy Markdown
Contributor

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants