Skip to content
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

Update to Material-UI v4 and fix lint issues #83

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

BenDiuguid
Copy link

  • Updated Material-UI to v4
  • Updated React to v16.8
  • Added dev dependencies to fix running yarn storybook locally
  • Fixed eslint warnings and errors (except 1 listed below)
mui-downshift/src/Input.js
  31:24  error  Do not use findDOMNode  react/no-find-dom-node

Let me know if I can fix up anything else! :)

@BenDiuguid
Copy link
Author

@cvanem Moving the discussion to this PR now, but the errors should be fixed, I should probably clean up the proptype stuff I have commented out, but basically I caused a bunch of errors with some of the assumptions I made around default props being incorrect previously.

Let me know what you need!

@BenDiuguid
Copy link
Author

Cleaned up the comments, let me know if I can do anything else! :)

@techniq
Copy link
Owner

techniq commented Aug 24, 2019

Quick feedback:

None of the selections appear to be working.

See items only and attempt to make a selection. Compared to current version. This isn't the only story, but one to look at

Errors in console

Outlined with custom adornments story shows these errors
image

@BenDiuguid Thanks for getting this PR moving, and hope to merge it soon. @cvanem let me know if you see anything as well. When it looks good, I'll cut a prerelease to get some more testing in apps before an official release.

@techniq
Copy link
Owner

techniq commented Aug 24, 2019

@BenDiuguid @cvanem
I spent some time this afternoon trying to get this closer to merging but ran into one error after another. Some is likely my own environment.

Getting this error in VSCode regarding the change in prettier to use babel instead of babylon.

[Error - 4:35:23 PM] ESLint stack trace:
[Error - 4:35:23 PM] Error: Couldn't resolve parser "babel"
    at resolveParser$1 (/Users/techniq/Documents/Development/open-source/mui-downshift/node_modules/prettier/index.js:7110:13)
    at normalize (/Users/techniq/Documents/Development/open-source/mui-downshift/node_modules/prettier/index.js:7202:19)
    at formatWithCursor (/Users/techniq/Documents/Development/open-source/mui-downshift/node_modules/prettier/index.js:10581:12)
    at /Users/techniq/Documents/Development/open-source/mui-downshift/node_modules/prettier/index.js:34924:15
    at Object.format (/Users/techniq/Documents/Development/open-source/mui-downshift/node_modules/prettier/index.js:34943:12)
    at Program (/Users/techniq/Documents/Development/open-source/mui-downshift/node_modules/eslint-plugin-prettier/eslint-plugin-prettier.js:415:45)
    at listeners.(anonymous function).forEach.listener (/Users/techniq/Documents/Development/open-source/mui-downshift/node_modules/eslint/lib/util/safe-emitter.js:45:58)
    at Array.forEach (<anonymous>)
    at Object.emit (/Users/techniq/Documents/Development/open-source/mui-downshift/node_modules/eslint/lib/util/safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector (/Users/techniq/Documents/Development/open-source/mui-downshift/node_modules/eslint/lib/util/node-event-generator.js:251:26)

Attempting to update all our dependencies (storybook especially) to clear up some console warnings about the old lifecycle methods...

image

but it's going to need more work...

image

I also needed to remove the old package.json and node_modules in the stories directory to fix an error.

I'm not sure when I'll get more time to work on this...

@BenDiuguid
Copy link
Author

I'll take a look at this when I get some time, but sanity check question, did you rm -rf node_modules/ && yarn

I remember resolving the babel parser issue when running eslint.

I'll double check running eslint from a clean yarn install when I get around to checking on the item selection issue.

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

Successfully merging this pull request may close these issues.

2 participants