Skip to content
This repository has been archived by the owner on Sep 17, 2021. It is now read-only.

undefined is not a function error from upgraded paths-js in many example app charts #62

Closed
marzolfb opened this issue Jan 29, 2017 · 17 comments

Comments

@marzolfb
Copy link
Contributor

It appears I broke many of the charts on the example app in Android specifically on d0de1e3 when upgrading paths-js from 0.3.4 to 0.4.5. I see the following error:

undefined is not a function (evaluating 'data.entries()[typeof Symbol==='function'?Symbol.iterator:'@@iterator']()')

on every type of chart other than the line charts (StockLine and SmoothLine)

@Jarda8
Copy link

Jarda8 commented Jan 30, 2017

I just updated the library and have the same error.

@marzolfb
Copy link
Contributor Author

marzolfb commented Jan 30, 2017

Thanks @Jarda8 for reaffirming that others are experience this.

I'll admit, I'm struggling a bit with this one. The "easy" thing to do would be to go back to the old version of paths-js (0.3.4) in effect before d0de1e3 (which upgraded paths-js 0.4.5) but then that would mean rolling back the feature of scaling the chart independent of the data with min/max and ability to specify rectangular regions on line charts.

I'll point out what I've discovered this far about the issue and see if others have any pointers to address the issue...

Just focusing on the pie chart source code in paths-js (though the pattern is repeated for other types of charts), this ES6-based line seems to be the start of the problem:

for (let [i, item] of data.entries())

When the paths-js build process invokes babel to turn this into "older javascript" (which is what you are pulling down when you do npm install paths-js), it translates that line into this line:

for (var _iterator = data.entries()[Symbol.iterator](), _step; !(_iteratorNormalCompletion = (_step = _iterator.next()).done); _iteratorNormalCompletion = true) {

The odd thing here is that I only see the error in the Android simulator and not the iOS simulator which makes me think that there are actually different Javascript engines in play on both simulators that are interpreting the babel-output javascript differently. But if you read the Javascript Environment page on the react-native docs, you see this:

On iOS simulators and devices, Android emulators and devices React Native uses JavaScriptCore which is the JavaScript engine that powers Safari. On iOS JSC doesn't use JIT due to the absence of writable executable memory in iOS apps.

which leads me to believe I should be seeing the same behavior on both emulators, but I don't???

I did also come across a seemingly-related issue here but none of the hints in there have led me to discover (yet) what needs to change with the paths-js transpiled code to make this not produce an error.

Ultimately once I (or someone else) figure out what the fix is, an issue/PR will need to be submitted to paths-js to get this issue addressed. Given that, it may be better to rollback to the old paths-js version and lose the extra functionality to get fully functional charts across the board.

Anyone else have any thoughts on anything I've mentioned above?

@marzolfb
Copy link
Contributor Author

It turns out that using the Babel polyfill solves the problem. Working on a PR to fix this now...

@jaredsmith
Copy link

Great catch, @marzolfb!

@tiaaaa123
Copy link

tiaaaa123 commented Feb 13, 2017

@marzolfb Unfortunately, it doesn't seem to fix the problem entirely. I'm still having this issue on Android. (Using a Pie Chart)
My version of react-native-pathjs-charts

    "react-native-pathjs-charts": "0.0.25",

@marzolfb marzolfb reopened this Feb 13, 2017
@marzolfb
Copy link
Contributor Author

marzolfb commented Feb 13, 2017

@tiaaaa123 - What is the output of this for you in your app:

npm list react-native react-native-svg paths-js

?

@tiaaaa123
Copy link

@marzolfb

├── UNMET PEER DEPENDENCY eslint-plugin-jsx-a11y@3.0.2
├── react-native@0.40.0 
└─┬ react-native-pathjs-charts@0.0.25
  ├── paths-js@0.4.5 
  └── react-native-svg@4.5.0 

@marzolfb
Copy link
Contributor Author

@tiaaaa123
Are you running the example app or your own app?
react-native-pathjs-charts is expecting react-native >= 0.41 - what happens if you upgrade?
Is it the Pie chart only or are you having issues with other chart types as well?

@tiaaaa123
Copy link

tiaaaa123 commented Feb 13, 2017

@marzolfb I'm running my own app, but with the example here : https://github.com/capitalone/react-native-pathjs-charts/wiki

Upgraded to :

├── UNMET PEER DEPENDENCY eslint-plugin-jsx-a11y@3.0.2
├── react-native@0.41.2 
└─┬ react-native-pathjs-charts@0.0.25
  ├── paths-js@0.4.5 
  └── react-native-svg@4.5.0 

EDIT: After trying them all, i found that the Tree, Pie, Bar, Scatterplot and the Radar are not working on android with the version previously said. Using only the data and the options of the wiki

@marzolfb
Copy link
Contributor Author

I'm having problems reproducing this in the example app included with this project. The data/options on the wiki site is taken directly from the example app in this repository. I'm not even sure what I can do to reproduce the UNMET PEER DEPENDENCY eslint-plugin-jsx-a11y@3.0.2 portion of the output you have above. If you can resolve your UNMET PEER DEPENDENCY, does the issue still persist?

@tiaaaa123
Copy link

I can try, but I'm pretty sure it has nothing to do with eslint-plugin-jsx-ally since eslint is simply a linter that check the code depending of styling rules...

@marzolfb
Copy link
Contributor Author

I agree. I was just trying to eliminate differences in hopes of reproducing the problem. Perhaps a different tact would be for you to try to run the example app with the setup you have. Do you have the same errors running the example app? If not, how does the example app setup compare to how your app is set up?

@TommasoPieroncini
Copy link

I just wanted to butt in and say that I'm having the same problem with the Pie and Chart code from the wiki, but the output to 'npm list react-native react-native-svg paths-js' for me is:

├── react-native@0.40.0
└─┬ react-native-pathjs-charts@0.0.25
├── paths-js@0.4.5
└── react-native-svg@4.5.0

@marzolfb
Copy link
Contributor Author

marzolfb commented Feb 14, 2017

@tiaaaa123 @TommasoPieroncini I looked back on the PR that fixed this issue (#64) and remembered the key was adding babel-polyfill as a dependency to the example app (and now I'm thinking I missed adding this as a dependency in the main library).

Also, you have to add an import to that dependency somewhere at the root of app (see what was done in the example app)

Have either/both of you added the babel-polyfill dependency to the apps you are working with?

Have either/both of you tried running the example app and does it not produce the same errors for you?

@marzolfb
Copy link
Contributor Author

After creating a separate app that tries to use this library (and not using the example app embedded in the project), I see the same error as @tiaaaa123 and @TommasoPieroncini. Rendering most of these charts has a dependency on babel-polyfill as I note above. I will start working on a PR to get this dependency added into the main library to truly resolve this problem (rather than just show how the dependency can be added in the example app as was done in #64). In the mean-time you can add babel-polyfill and then add the import to your app entrypoint somewhere to workaround this for now.

@tiaaaa123
Copy link

it works!
however, I don't think it's a good idea to import a dev dependencies... I could add a rule to my eslint to ignore the no-extraneous-dependencies, but I won't. I will let the issue as it is for now.

But yeah, importing babel-polyfill works. First chart librairy to work in react-natine for android!
Please, tell me when your PR is online! 👍

@TommasoPieroncini
Copy link

Success! Thank you for your help @marzolfb , love to see authors involved in solving issues. babel-polyfill did it for me. Looking forward to seeing any progress on this library. I feel you @tiaaaa123 , this is the first graph library I've gotten to work on both platforms on react-native. I'll be waiting for that PR as well!

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

No branches or pull requests

5 participants