Upgrade all dependencies and remove transpilation step #49

SimenB · 2018-09-23T20:50:34Z

This upgrades all dependencies, dropping node 4 support, and bringing in the improvement in create-jest-runner we need for the watch plugin.

It also upgrades the dependency on Eslint, not sure about that one? Feels like it should be a peer dependency anyways, bringing our own is... weird. And the way we load it breaks the assumptions lerna/yarn makes in a monorepo as well (it looks for closest node_modules and expect to find eslint there, instead of walking up the tree to the hoisted one.)

EDIT: Ah, that's #41

EDIT2: Oh, removing the babel step allows us to install from github which is nice 🙂

SimenB · 2018-09-23T20:51:23Z

@rogeliog I noticed #48 after I started work on this, so I submitted it anyways 🙂

SimenB · 2018-09-23T20:57:14Z

package.json

-    "build": "babel src --ignore **/*.test.js,integration -d build",
-    "prepublish": "yarn build",
-    "format": "prettier --single-quote --trailing-comma all --write \"!(build)/**/*.js\""
+    "lint": "eslint . --config ./.eslintrc.json --no-eslintrc",


had to do this to avoid loading https://github.com/jest-community/jest-runner-eslint/blob/master/integrationTests/__fixtures__/custom-parser/.eslintrc.json

ljharb

I’m a big -1 on removing the babel step; we should be using the env preset forever and just changing the targets. Installing from github is antipattern anyways.

ljharb · 2018-09-23T21:09:13Z

(Absolutely both jest and eslint should be peer dependencies, with explicit ranges)

SimenB · 2018-09-23T21:13:46Z

The only thing we do syntax wise which doesn't work on node 6 is trailing commas. What benefit does it provide? I suppose we could replace our Object.assign with spread and use async-await? If that's something we want to do (or at least have the freedom to do) I'm not opposed to adding it back in. I did it mostly since it simplified the repo a bit, I don't really care either way.

Installing from github is antipattern anyways.

Fair enough, in this case I wanted to test out the watch plugin branch. Solution was just a yarn/npm link away, so no biggie. (funnily it would have worked if main pointed to src as node 8 runs the code base without issues)

(Absolutely both jest and eslint should be peer dependencies, with explicit ranges)

Cool, we can do that before a release then, bunch up the breaking changes

ljharb · 2018-09-23T22:07:40Z

That at the moment the only syntax a project uses is supported isn’t knowledge that humans should have to think about; that’s what the env preset covers. We should be able to write modern JS at any point and not be tied to v8’s feature release schedule (or node’s v8 update schedule).

In other words, it simplifies the repo’s scripts but ends up making the cognitive overhead for maintainers more complex.

SimenB · 2018-09-24T08:01:03Z

Re-added babel (although using babel 7 instead of 6). Difference in output after looking at the diff is that we now use arrow functions and const over var

ljharb · 2018-09-24T13:19:53Z

CHANGELOG.md

@@ -3,12 +3,14 @@
 ## 0.6.0

 ### Fixes
-* cli options rules should be passed as an object instead of array of strings ([#39](https://github.com/jest-community/jest-runner-eslint/pull/39))


Is this just running prettier on markdown files? Can we revert the unrelated changes? Prettier makes markdown uglier imo, except for tables.

This is prettier, yeah.

What's uglier about it? It's just swapping * for -.

I'm of the mind to kust let prettier do its thing rather than caring. Just set and forget

This project doesn't currently use prettier; but also yes, the - is less readable in raw form than the *.

imo, a "set and forget" mentality abdicates responsibility for ensuring things are readable :-/

We do use prettier; we have a prettier in package.json, a prettierignore and the eslint plugin for it - all on master

"prettier in package.json" is added by this PR.

I do see npm run format; either way, i'd expect people to have editors run eslint autofix, but not to blindly run prettier on all files, and i'd prefer not to run it on markdown files.

jest-runner-eslint/package.json

Line 41 in c811667

"prettier": "1.5.3"

That's master?

ah, i see the dep - but not the config, which is what confused me.

In master, prettier only runs on JS files - i'd prefer to keep it that way.

fair enough 🙂 I'll keep the changes it made though

:-/ the changes it made are what i have a problem with.

SimenB · 2018-09-28T07:06:33Z

.travis.yml

@@ -2,5 +2,4 @@ language: node_js
 node_js:
  - '10'
  - '8'
-  - '7'


eslint refuses to install: error eslint@5.6.0: The engine "node" is incompatible with this module. Expected version "^6.14.0 || ^8.10.0 || >=9.10.0".

We could ignore engines, but I don't think that's better than dropping a version of Node that has been EOL since summer 2017

it's fine not to run on EOL never-LTS versions of node imo.

Yeah, good point on differentiating between what was LTS at one point and what was not

SimenB requested review from ljharb and rogeliog September 23, 2018 20:50

SimenB commented Sep 23, 2018

View reviewed changes

ljharb requested changes Sep 23, 2018

View reviewed changes

ljharb reviewed Sep 24, 2018

View reviewed changes

rogeliog approved these changes Sep 25, 2018

View reviewed changes

SimenB force-pushed the bumpybumpp branch from ab64529 to a860a08 Compare September 27, 2018 07:28

rogeliog mentioned this pull request Sep 28, 2018

Watch plugin for toggling --fix #53

Merged

SimenB force-pushed the bumpybumpp branch from a860a08 to 4de0198 Compare September 28, 2018 06:52

SimenB mentioned this pull request Sep 28, 2018

Set eslint as a peer dependency #54

Closed

SimenB force-pushed the bumpybumpp branch from 4de0198 to 88f715d Compare September 28, 2018 06:59

upgrade all dependencies

7e9cb80

SimenB force-pushed the bumpybumpp branch from 88f715d to 7e9cb80 Compare September 28, 2018 07:05

SimenB commented Sep 28, 2018

View reviewed changes

SimenB merged commit 3fc5ab8 into master Sep 28, 2018

SimenB deleted the bumpybumpp branch September 28, 2018 07:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade all dependencies and remove transpilation step #49

Upgrade all dependencies and remove transpilation step #49

SimenB commented Sep 23, 2018 •

edited

SimenB commented Sep 23, 2018

SimenB Sep 23, 2018

ljharb left a comment

ljharb commented Sep 23, 2018

SimenB commented Sep 23, 2018 •

edited

ljharb commented Sep 23, 2018

SimenB commented Sep 24, 2018 •

edited

ljharb Sep 24, 2018

SimenB Sep 25, 2018

ljharb Sep 25, 2018

SimenB Sep 25, 2018 •

edited

ljharb Sep 26, 2018

SimenB Sep 26, 2018

ljharb Sep 26, 2018

SimenB Sep 28, 2018 •

edited

ljharb Sep 28, 2018

SimenB Sep 28, 2018

ljharb Sep 28, 2018

SimenB Sep 28, 2018

Upgrade all dependencies and remove transpilation step #49

Upgrade all dependencies and remove transpilation step #49

Conversation

SimenB commented Sep 23, 2018 • edited

SimenB commented Sep 23, 2018

Choose a reason for hiding this comment

ljharb left a comment

Choose a reason for hiding this comment

ljharb commented Sep 23, 2018

SimenB commented Sep 23, 2018 • edited

ljharb commented Sep 23, 2018

SimenB commented Sep 24, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SimenB Sep 25, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SimenB Sep 28, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SimenB commented Sep 23, 2018 •

edited

SimenB commented Sep 23, 2018 •

edited

SimenB commented Sep 24, 2018 •

edited

SimenB Sep 25, 2018 •

edited

SimenB Sep 28, 2018 •

edited