Page MenuHomePhabricator

Migrate tslint to eslint
AbandonedPublic

Authored by sashank on Jul 17 2019, 9:22 PM.

Details

Reviewers
bengotow
Group Reviewers
Restricted Project
Test Plan

eslint

Diff Detail

Repository
R1 dagster
Branch
eslint (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

sashank updated this revision to Diff 3000.Jul 17 2019, 9:22 PM
sashank created this revision.

Remove extra irrelevant changes

bengotow accepted this revision.Jul 18 2019, 7:42 PM
bengotow added a subscriber: bengotow.
bengotow added inline comments.
js_modules/dagit/.eslintrc.js
5

Looks correct to me! Thanks for upgrading this to use eslint. I'm actually kind of surprised moving to the newer version of typescript's rules didn't trigger a whole bunch of new linter errors. Guess we're in good shape!

This revision is now accepted and ready to land.Jul 18 2019, 7:42 PM
sashank added a reviewer: Restricted Project.Jul 18 2019, 8:33 PM
sashank updated this revision to Diff 3046.Jul 18 2019, 8:54 PM

Add allowSyntheticDefaultImports flag to tsconfig

Any ideas on how to get the jest tests to pass? It's complaining about the codemirror import but I can't figure out how to fix it

sashank requested review of this revision.Jul 18 2019, 8:55 PM
max added a subscriber: max.Jul 19 2019, 4:42 PM
yarn run v1.16.0
$ yarn run build && cd ../../python_modules/dagit/dagit && rm -rf webapp && mkdir -p webapp && cp -r ../../../js_modules/dagit/build ./webapp/ && mkdir -p webapp/build/vendor/graphql-playground && cp ../../../js_modules/dagit/node_modules/graphql-playground-react/build/static/css/index.css webapp/build/vendor/graphql-playground/index.css && cp ../../../js_modules/dagit/node_modules/graphql-playground-react/build/favicon.png webapp/build/vendor/graphql-playground/favicon.png && cp ../../../js_modules/dagit/node_modules/graphql-playground-react/build/static/js/middleware.js webapp/build/vendor/graphql-playground/middleware.js
$ react-scripts-ts build
 
There might be a problem with the project dependency tree.
It is likely not a bug in Create React App, but something you need to fix locally.
 
The react-scripts-ts package provided by Create React App requires a dependency:
 
  "eslint": "5.6.0"
 
Don't try to install it manually: your package manager does it automatically.
However, a different version of eslint was detected higher up in the tree:
 
  /workdir/js_modules/dagit/node_modules/eslint (version: 6.0.1)
 
Manually installing incompatible versions is known to cause hard-to-debug issues.
To fix the dependency tree, try following the steps below in the exact order:
 
  1. Delete package-lock.json (not package.json!) and/or yarn.lock in your project folder.
 
  2. Delete node_modules in your project folder.
 
  3. Remove "eslint" from dependencies and/or devDependencies in the package.json file in your project folder.
 
  4. Run npm install or yarn, depending on the package manager you use.
 
In most cases, this should be enough to fix the problem.
If this has not helped, there are a few other things you can try:
 
  5. If you used npm, install yarn (http://yarnpkg.com/) and repeat the above steps with it instead.
     This may help because npm has known issues with package hoisting which may get resolved in future versions.
 
  6. Check if /workdir/js_modules/dagit/node_modules/eslint is outside your project directory.
     For example, you might have accidentally installed something in your home folder.
 
  7. Try running npm ls eslint in your project folder.
     This will tell you which other package (apart from the expected react-scripts-ts) installed eslint.
 
If nothing else helps, add SKIP_PREFLIGHT_CHECK=true to an .env file in your project.
That would permanently disable this preflight check in case you want to proceed anyway.
 
P.S. We know this message is long but please read the steps above :-) We hope you find them helpful!
sashank planned changes to this revision.Jul 19 2019, 11:41 PM

@alangenfeld is this resolved by D674?

ya this can get abandoned

sashank abandoned this revision.Thu, Jul 25, 4:18 PM

Fixed as part of different diff