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

React 16 #144

Closed
wants to merge 6 commits into from
Closed

React 16 #144

wants to merge 6 commits into from

Conversation

mathisonian
Copy link
Member

This updates the project to use react-16 (targeted against the idyll-doc-updates branch). Its a relatively minor change, but will be good to get in, and will potentially allow us to take advantage of the new error boundary features.

There are some warnings being thrown by Victory (only in dev - not production) but they are aware and say they'll fix these in the next release FormidableLabs/victory#724

@bclinkinbeard
Copy link
Contributor

I definitely want to get onto 16 but enzyme doesn't support it yet.

@mathisonian
Copy link
Member Author

Ah yeah, looks like we'll need to follow enzymejs/enzyme#1007 and enzymejs/enzyme#928. Looks like it won't be supported until they release v3

@mathisonian mathisonian changed the base branch from idyll-doc-updates to master September 13, 2017 02:59
@bclinkinbeard
Copy link
Contributor

Updated the other packages and rebased against master. @mathisonian if you're good with this click the button :)

@mathisonian
Copy link
Member Author

One of the cli tests is failing with TypeError: React.createClass is not a function. It looks like there's a little more cleanup to do to get that working, then I'm good to merge

@bclinkinbeard
Copy link
Contributor

Where did you see that particular error? I think I'm only seeing Enzyme-related failures.

Also, why are Travis and Appveyor both running twice?

@mathisonian
Copy link
Member Author

In the travis build - it seems like the react-micro-bar-chart doesn't like react 16

    console.error src/index.js:102
      TypeError: React.createClass is not a function
          at Object.<anonymous> (/home/travis/build/idyll-lang/idyll/packages/idyll-cli/test/test-project/src/node_modules/react-micro-bar-chart/dist/index.js:10:24)
          at Runtime._execModule (/home/travis/build/idyll-lang/idyll/node_modules/jest-runtime/build/index.js:446:13)
          at Runtime.requireModule (/home/travis/build/idyll-lang/idyll/node_modules/jest-runtime/build/index.js:294:14)
          at Runtime.requireModuleOrMock (/home/travis/build/idyll-lang/idyll/node_modules/jest-runtime/build/index.js:364:19)
          at /home/travis/build/idyll-lang/idyll/packages/idyll-cli/src/pipeline/parse.js:206:29
          at Array.forEach (native)
          at Object.<anonymous>.exports.getHTML (/home/travis/build/idyll-lang/idyll/packages/idyll-cli/src/pipeline/parse.js:204:27)
          at /home/travis/build/idyll-lang/idyll/packages/idyll-cli/src/pipeline/index.js:42:21
          at tryCatcher (/home/travis/build/idyll-lang/idyll/node_modules/bluebird/js/release/util.js:16:23)
...

It is their default setting to run twice like that, but we can probably change it to not

@mathisonian
Copy link
Member Author

I just tweaked some settings - hopefully they'll only run once for PRs now

@bclinkinbeard
Copy link
Contributor

I removed react-micro-bar-chart

@mathisonian
Copy link
Member Author

I'm still seeing errors, now from reactable (looks like it is documented here glittershark/reactable#381 - but that library hasn't been updated in a while). I wonder if its worth trying to patch that, or just finding a better maintained table component.

I'll take a look at options a little later on today

@bclinkinbeard
Copy link
Contributor

@mathisonian
Copy link
Member Author

That one is apparently not maintained either? facebookarchive/fixed-data-table#468

Whats the deal with table libraries? (╯°□°)╯︵ ┻━┻

@bclinkinbeard
Copy link
Contributor

bclinkinbeard commented Sep 14, 2017 via email

@mathisonian
Copy link
Member Author

mathisonian commented Sep 14, 2017

This one looks like it has potential, and a simple but still powerful API which is a plus

edit: forgot the link https://github.com/react-tools/react-table

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

Successfully merging this pull request may close these issues.

None yet

2 participants