Skip to content
This repository has been archived by the owner on May 20, 2020. It is now read-only.

Upgrade 2018 02 06 #11

Merged
merged 4 commits into from
Feb 23, 2018
Merged

Upgrade 2018 02 06 #11

merged 4 commits into from
Feb 23, 2018

Conversation

karfau
Copy link
Member

@karfau karfau commented Feb 6, 2018

Upgrades

Major Update Potentially breaking API changes. Use caution.

  • typescript devDep 2.5.3 ❯ 2.7.1
    >=2.6.1 as peerDependency since tslint-react requires it
  • @types/node devDep 8.0.46 ❯ 9.4.0
  • ts-node devDep 3.3.0 ❯ 4.1.0
    >=3.2.0 as peerDependency since it build/test worked with those versions
    and no dependency requires a higher version

Minor Update New backwards-compatible features.

  • node >=8.1.4(nvm + package.json) /8.2.0(circleci) ❯ >=8.2.0(package.json)/ 8.9.0(nvm+circleci)
    npm >=5.0.3>=5.2.0(package.json)
    the versions now required by package.json are the first ones to provide npx, which we make heavy use of in our scripts
  • tslint-react 3.2.0 ❯ 3.4.0

Patch Update Backwards-compatible bug fixes.

  • tslint-microsoft-contrib 5.0.1 ❯ 5.0.2

Other changes

difference in dependencies

I did some research and spent some thoughts on how the dependencies should be set up in this package. I documented my thoughts in the new README.md.

Since this requires to make changes to package.json for someone using this package It is a breaking change, so I increased the version number to 0.1.0.

problem upgrading tslint

When trying to build our custom rules with a newer version of tslint, it complained the following:

node_modules/tslint/lib/runner.d.ts(60,27): error TS1209: Ambient const enums are not allowed when the '--isolatedModules' flag is provided.
npm ERR! code ELIFECYCLE
npm ERR! errno 2
npm ERR! [email protected] build: `tsc -p .`
npm ERR! Exit status 2

I could't find to any useful information about it, at least not in the expected scope/dimensions.
I'm filed issue palantir/tslint#3700 to find out what is going on.

more rule changes

Since the PR is already quite big, I want to add the proposed rule changes in a later PR.

@karfau karfau requested review from saurabh2590, lulu-berlin and a team February 6, 2018 21:34
* `tslint-react`:
- [extending ruleset](https://github.com/palantir/tslint-react/blob/master/tslint-react.json)
- [available rules and options](https://github.com/palantir/tslint-react#rules)

Copy link
Contributor

Choose a reason for hiding this comment

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

🥇 for documenting in detail.

"main": "tslint.json",
"files": [
"dist"
],
"engines": {
"node": ">=8.1.4",
"npm": ">=5.0.3"
"node": ">=8.2.0",
Copy link
Contributor

@saurabh2590 saurabh2590 Feb 16, 2018

Choose a reason for hiding this comment

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

Question: Since .nvmrc sets node version to 8.9.0 and it's LTS version, could we also set this to be same?

Copy link
Member Author

Choose a reason for hiding this comment

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

in .nvmrc we are setting the version to easily use for development.
In package.json we are setting the minimal required version for installing/using the lib.
So as long as we don't know about any incompatibilities I think it is fine to stick the the minimum that we need (in this case we need it because we are using npx which was first available in node 8.2/npm 5.2.
Which is also the reason we are still using that version on CIrcleCI to prove it still works with this version.
I also tried to explain this in the PR description, but maybe it was hard to spot there.

Copy link
Contributor

@saurabh2590 saurabh2590 left a comment

Choose a reason for hiding this comment

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

Minor question on keep the same node version in package.json as in .nvmrc file.
No other concerns.

@karfau karfau merged commit bd4bb37 into master Feb 23, 2018
@karfau karfau deleted the upgrade/2018-02-06 branch February 23, 2018 10:15
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