Skip to content
This repository has been archived by the owner on Dec 1, 2019. It is now read-only.

add typescript as a peer dependency #235

Closed
wants to merge 1 commit into from
Closed

add typescript as a peer dependency #235

wants to merge 1 commit into from

Conversation

punmechanic
Copy link

This will prevent issues like #226 and #229 from being raised again
(hopefully).

awesome-typescript-loader 2.2.3 onwards broke
compatibility with TypeScript 2.0 and some people (including myself)
depended on it. Unfortunately, it failed silently, leaving a rather
cryptic error. TypeScript's dependency was updated as a devDependency on
this package to 2.0.2, however TypeScript itself has never been
installed as a peerDependency.

Adding TypeScript as a peerDependency will alert consumers of this
package when they install a version of TypeScript that is not supported
by this plugin.

Of course, perhaps >= 2.0.2 is too weak a constraint, but it's better
than none.

Here's what will be displayed if you install a version below the required version (in this example I installed TypeScript 2.0.0)

example console output

This will prevent issues like #226 and #229 from being raised again
(hopefully).

`awesome-typescript-loader` 2.2.3 onwards broke
compatibility with TypeScript 2.0 and some people (including myself)
depended on it. Unfortunately, it failed silently, leaving a rather
cryptic error. TypeScript's dependency was updated as a devDependency on
this package to 2.0.2, however TypeScript itself has never been
installed as a peerDependency.

Adding TypeScript as a peerDependency will alert consumers of this
package when they install a version of TypeScript that is not supported
by this plugin.

Of course, perhaps `>= 2.0.2` is too weak a constraint, but it's better
than none.

Signed-off-by: Dan Pantry <devnull@danpantry.me>
@punmechanic
Copy link
Author

punmechanic commented Sep 15, 2016

There seems to be a bug with your CI runner and Babel plugins. :( that CI failure has nothing to do with this change.

Note that you may wish to alter the peerDependency version (I'm not sure what versions you support).

@s-panferov s-panferov closed this Feb 24, 2018
@punmechanic
Copy link
Author

punmechanic commented Feb 25, 2018

Hey @s-panferov, I'm glad you fixed the issue, but a little disappointed you merged a PR by an existing contributor (#536) and closed this PR without saying anything.

In future I'd encourage you to leave feedback on why a specific PR wasn't merged (especially as it has been open for so long).

I understand it's a trivial fix, but it does feel kinda bad :)

@s-panferov
Copy link
Owner

@danpantry sorry for this. I have very limited time to maintain this project and it was just easier for me to merge more common request. This fix appeared quite trivial to me to accept the easy path. Thanks for your contribution anyway!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants