Skip to content

Remove typescript and tslint from peerDependencies#109

Closed
marcofugaro wants to merge 2 commits into
TypeStrong:masterfrom
marcofugaro:master
Closed

Remove typescript and tslint from peerDependencies#109
marcofugaro wants to merge 2 commits into
TypeStrong:masterfrom
marcofugaro:master

Conversation

@marcofugaro

@marcofugaro marcofugaro commented Mar 29, 2018

Copy link
Copy Markdown

Was also discussed in #102,

Our problem with those peerDeps, is that we optionally use typescript in our build process, so just by having this package in the dependencies, it throws the error typescript is required.

screen shot 2018-03-29 at 11 26 03

It should be like ts-loader, typescript is required but it's up to the user to install it.

Thre README is fine since it says it explicitly to install also typescript.

Closes #102

@piotr-oles

Copy link
Copy Markdown
Collaborator

If we want to remove it from peerDeps, we should add some checks (for typescript package and it's version) in the plugin code to inform user that typescript is not available and he has to install it to use this plugin. :)

@SimonSchick

Copy link
Copy Markdown
Contributor

@nickserv

nickserv commented Aug 8, 2018

Copy link
Copy Markdown
Contributor

Unfortunately optionalDependencies don't work with peerDependencies, they have to be direct dependencies of this package and not its dependent, which isn't helpful.

@sebinsua

sebinsua commented Oct 18, 2018

Copy link
Copy Markdown

This isn't correct.

peerDependencies will not cause an error about a module not existing. All it does is cause a warning to be printed during installation telling you what version range is expected. It's purely informational to help developers understand what version of the TypeScript compiler is supported. It's your choice whether you then install this.

If you want to not get runtime errors, you will need to write something like this.

let typescript;
try {
  typescript = require('typescript');
} catch (err) {
  console.warn('TypeScript needs to be installed for fork-ts-checker-webpack-plugin to do anything.');
}

// ...somewhere deep within the plugin...

if (typescript) {
  // Conditionally decide what to do if it is installed here.
}

@glen-84

glen-84 commented Dec 12, 2018

Copy link
Copy Markdown

All it does is cause a warning to be printed during installation telling you what version range is expected.

... and this when using npm list --depth=0:

npm ERR! peer dep missing: tslint@^4.0.0 || ^5.0.0, required by fork-ts-checker-webpack-plugin@0.5.1

I'm not using the tslint integration, and it even requires setting tslint: true, so I'm not sure why it's listed as a peer dependency.

@sebinsua

sebinsua commented Dec 12, 2018

Copy link
Copy Markdown

If you remove the peerDependency you no longer have a way to inform the user of what version range of a dependency is supported.

I think what you're implicitly asking for is optional peerDependencies. This feature is in discussion.

@marcofugaro

Copy link
Copy Markdown
Author

Closing because it was done in #201

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.

6 participants