-
-
Notifications
You must be signed in to change notification settings - Fork 245
Use minimum peer dependencies #134
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
Use minimum peer dependencies #134
Conversation
Looks good to me. What do you think @piotr-oles? |
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.
Please add also cases in .travis.yml for TypeScript 2.x and TypeScript 3.x (using ENV variable - like it's done for ts-loader, ts-lint and vue-loader)
package.json
Outdated
"tslint": "^4.0.0 || ^5.0.0", | ||
"typescript": "^2.1.0", | ||
"webpack": "^2.3.0 || ^3.0.0 || ^4.0.0" | ||
"tslint": ">= 4.0.0", |
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.
It's a bold assumption that next major releases of tslint, typescript and webpack will be compatible with this plugin :) Please add typescript ^3.0.0 instead
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.
I agree. Done.
I'm not sure if using ENV variables like that for TypeScript would be safe in the future. I updated TypeScript in the package's own devDependencies so you can take advantage of new syntax features, along with the improved project support and error messages. If you change source code to have syntax or config options that are only supported in TS 3, the TS 2 env will fail. Are you sure you still want to try that? That would mean this package cannot use TS 3 features in its own TS files without changing the build setup further to separate TS transpilation from integration testing other versions of TS in other packages. |
Good point - let's keep test as they are currently :) Thank you for your work! |
Fixes #133.
I have used TypeScript 3 with this loader in a CRA-TS app and ran the build and test steps test no issues so far. Let me know if there's anything else we need to change or test.