Skip to content

Add repository and dependency in package.json #24

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 2 commits into from
Aug 26, 2018

Conversation

Eskibear
Copy link
Member

See #21

  • The entry "repository" must be specified for vsce to resolve absolute links in .md files.
  • The package "uuid" has to be explicitly added into dependency.

@Eskibear Eskibear added this to the V 0.1.0 milestone Aug 23, 2018
@Eskibear Eskibear self-assigned this Aug 23, 2018
@Eskibear Eskibear requested a review from kdvolder August 23, 2018 06:34
package.json Outdated
"redhat.java"
],
"dependencies": {
"uuid": "^3.3.2"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why does the @types/uuid under devDependencies have a different version than the uuid under dependencies?

@Eskibear
Copy link
Member Author

Why does the @types/uuid under devDependencies have a different version than the uuid under dependencies?

Technically, the versions of the 2 packages are independent.
You know that eventually TS code is compiled into JS code, and @types/uuid just provides TypeScript type definition for the package uuid.
@types/uuid v3.4.3 was published 10 months ago, uuid v3.3.2 was published 2 months ago. Obviously the types are outdated if there's any change of uuid APIs during this period of time.

Yes ideally following the semantic versioning, they are supposed to match. But in fact there are no mechanism to ensure that. See this anwser, unfortunately it's a known issue over the TS community.

devDependencies v.s. dependencies

Packages listed in "devDependencies" will not be packed into the .vsix package, but those in "dependencies" will. So only "dependencies" are available in runtime, that's why uuid should be added there.

Workaround to bypass TS type check

More to say, sometimes you may want to use a third party JS library, but there is only an outdated "@types" package (potential gaps in types definition because of breaking changes) or even no corresponding "@types" package. For example, without install '@types/uuid' , following import statement will cause TS compile error.

import * as uuid from 'uuid';  // the package "@types/uuid" is needed for tsc to compile. 
tsc -p ./

error TS7016: Could not find a declaration file for module 'uuid'. 'C:/Users/yanzh/Work/vscode-spring-boot-dashboard/node_modules/uuid/index.js' implicitly has an 'any' type.
  Try `npm install @types/uuid` if it exists or add a new declaration (.d.ts) file containing `declare module 'uuid';`

Then you might want to add "//@ts-ignore" above the statement, or set "noImplicitAny" to "false" in your "tsconfig.json" file .

Refs:
microsoft/TypeScript#3019
microsoft/TypeScript#15031

Copy link
Collaborator

@kdvolder kdvolder left a comment

Choose a reason for hiding this comment

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

Thanks for the clarification. I was assuming that the @types package would follow the same versioning as the js package its type describe as that would seem the most logical approach. But now I understand that isn't necessarily the case.

I did understand that uuid needs to be added to dependencies and why. In fact I thought I already did when I adopted that package to generate unique ids for the classpath callbacks. But it looks like I dropped the ball there somehow. Thanks for fixing that packaging issue.

I vote to merge this PR.

@Eskibear Eskibear merged commit a7914ad into microsoft:master Aug 26, 2018
@Eskibear Eskibear deleted the pr_meta branch August 26, 2018 12:57
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.

2 participants