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

Support node 4.x and higher #369

Closed
shadeglare opened this issue Feb 13, 2016 · 5 comments
Closed

Support node 4.x and higher #369

shadeglare opened this issue Feb 13, 2016 · 5 comments

Comments

@shadeglare
Copy link

Some parts of code are not compatible with node 4.x and higher. Like inappropriate using the Buffer length property which is now readonly. Any possibilities that could be resolved?

@shadeglare shadeglare changed the title Support node > 4.x Support node 4.x and higher Feb 13, 2016
@pietersv
Copy link
Contributor

Aha. To confirm I just ran the (amazingly thorough) test suite and see some of the tests of .xlsx failing on node 4.1 on a clean clone.

$> make test

1) should parse test files AutoFilter.xlsb [.xlsb]:
     Error: Unexpected record 0 BrtRowHdr
      at hopper_sty (/Users/me/Dev/js-xlsx/xlsx.js:4915:41)
      at recordhopper (/Users/pieter/Dev/js-xlsx/xlsx.js:1693:6)
      at parse_sty_bin (/Users/me/Dev/js-xlsx/xlsx.js:4856:2)
      at parse_sty (/Users/me/Dev/js-xlsx/xlsx.js:8436:67)
      at parse_zip (/Users/me/Dev/js-xlsx/xlsx.js:11176:26)
      at read_zip (/Users/me/Dev/js-xlsx/xlsx.js:11380:9)
      at Object.readSync [as read] (/Users/me/Dev/js-xlsx/xlsx.js:11396:11)
      at Context.<anonymous> (/Users/me/Dev/js-xlsx/test.js:193:20)
...

and if I comment out tests of xlsb files then get

  1) should parse test files biff5/NumberFormatCondition.xls:
     TypeError: Cannot set property length of [object Object] which has only a getter
      at sbcs_d (/Users/me/Dev/js-xlsx/dist/cpexcel.js:909:20)
      at Object.decode (/Users/me/Dev/js-xlsx/dist/cpexcel.js:1197:40)
      at _gc2 (/Users/me/Dev/js-xlsx/xlsx.js:28:24)
      at Buffer.ReadShift [as read_shift] (/Users/me/Dev/js-xlsx/xlsx.js:1623:13)
      at parse_ShortXLUnicodeString (/Users/me/Dev/js-xlsx/xlsx.js:3082:21)
      at Object.parse_Font [as f] (/Users/me/Dev/js-xlsx/xlsx.js:3470:13)
      at slurp (/Users/me/Dev/js-xlsx/xlsx.js:9229:11)
      at parse_workbook (/Users/me/Dev/js-xlsx/xlsx.js:9350:15)
      at parse_xlscfb (/Users/me/Dev/js-xlsx/xlsx.js:9827:27)
      at readSync (/Users/me/Dev/js-xlsx/xlsx.js:11391:11)
     ...

@xxScorpion
Copy link

node_modules\xlsx\dist\cpexcel.js:909
mdb.length = 2 * len;
^
TypeError: Cannot set property length of [object Object] which has only a getter

To correct you need "mdb = new Buffer(mdl)" to change "mdb = Array(mdl)"
Can anyone make corrections to the repository ?

@blakehagen
Copy link

It appears @fluciotto has a robust fix for this issue as opposed to just commenting out the line as others have. Is this something that you will try to merge into master or should I just use @fluciotto's fork?

@pietersv
Copy link
Contributor

pietersv commented Dec 7, 2016

@blakehagen that's fantastic news. I'd prefer to merge, will have time next week. If your work is more urgent, use the other fork, and we can connect in a bit.

@reviewher
Copy link
Contributor

Fixed in 0.8.1, test suite currently runs against node versions 0.8 0.9 0.10 0.12 4.2 5 6 7

If you find any issues where something works in the latest iteration of one node version but not another, it is a bug and please open a new 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

No branches or pull requests

5 participants