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

Cannot read property 'charAt' of undefined #1142

Closed
JasonBrannon opened this issue May 16, 2016 · 20 comments
Closed

Cannot read property 'charAt' of undefined #1142

JasonBrannon opened this issue May 16, 2016 · 20 comments

Comments

@JasonBrannon
Copy link

JasonBrannon commented May 16, 2016

npm --version: 3.8.6
node --version: 6.1.0

When running dc.js$ grunt server receive the following error output:

dc.js$ grunt server
Running "concat:js" (concat) task
Source map dc.js.map created.
File dc.js created.

Running "uglify:jsmin" (uglify) task

1 sourcemap created.
1 file created.

Running "cssmin:main" (cssmin) task

1 file created. 4.87 kB → 3.39 kB

Running "copy:dc-to-gh" (copy) task
Copied 9 files

Running "jsdoc:dist" (jsdoc) task
Documentation generated to /Users/jason/Desktop/WhitelistUX/dc.js-develop/web/docs/html
Running "jsdoc2md:dist" (jsdoc2md) task

writing web/docs/api-latest.md
Fatal error: Cannot read property 'charAt' of undefined

@gordonwoodhull
Copy link
Contributor

gordonwoodhull commented May 16, 2016

Thanks for the report - looks like one of our zillion dev dependencies has been updated and broke things.

I was not able to reproduce until I deleted node_modules and reinstalled and now I see it. Hope to take a look later today.

@JasonBrannon
Copy link
Author

Thanks for update. For my purposes to run the examples, I changed the following in Gruntfile.js

grunt.registerTask('server', ['docs', 'fileindex', 'jasmine:specs:build', 'connect:server', 'watch:jasmine-docs']);
to
grunt.registerTask('server', ['fileindex', 'jasmine:specs:build', 'connect:server', 'watch:jasmine-docs']);

@gordonwoodhull
Copy link
Contributor

Great, glad you found a workaround for now!

@mtraynham
Copy link
Contributor

mtraynham commented May 23, 2016

Seems to be related to some latest changes to jsdoc2md and the @memberof clause. It breaks when you have a nested property dot-notation in the @memberof clause.

    /**
     * @memberof dc.barChart
     */

Simply doing * @memberof barChart won't work though. I was able to get this to work after downgrading grunt-jsdoc2md to 0.4.5.
Maybe @75lb can comment, I believe he's been around here before.

Edit Ahh, looks like you guys are way ahead of me.

@gordonwoodhull
Copy link
Contributor

Thanks @mtraynham, I filed an issue on grunt-jsdoc-to-markdown and @75lb tracked it down to a bug in a second-level dependency of grunt-gh-pages. I didn't know that was possible.

I just filed an issue minutes ago on grunt-gh-pages asking them to upgrade q-io to 2 (see link above). Testing out an npm-shrinkwrap now.

@gordonwoodhull
Copy link
Contributor

gordonwoodhull commented May 23, 2016

Shrinkwrap fixes it! Will close shortly. (Just forcing q-io@2, although shrinkwrap forces you to also nail down the immediate sibling dependencies. :-/)

@mtraynham
Copy link
Contributor

mtraynham commented May 23, 2016

Yeah... node's builtins are still global space, so assigning a new function to Array.prototype would do it. Cool that shrinkwrap works! Since I was curious, I did try upgrading all the devDependencies to latest, and everything still works (except for this bug and jshint's latedef needed to be false). Did you want to bump everything up as well?

@gordonwoodhull
Copy link
Contributor

I bumped a couple of them, but certainly a PR with the rest would be helpful. I'm going to do one more beta today with this and the clip path bug you pointed out, and then hopefully do 2.0 later in the week!

@gordonwoodhull
Copy link
Contributor

Is there an automated way to try all the latest, or do you have to look up each package?

@mtraynham
Copy link
Contributor

mtraynham commented May 23, 2016

Awesome news on 2.0!

I use npm-check-updates installed globally and then run ncu -u to update the package.json file. As a secondary update, I removed all the .x versions, since those are accomplished by ~ for patch updates or ^ for minor updates. Looks like the package emu is no longer used as well:

  "dependencies": {
    "crossfilter2": "~1.3",
    "d3": "^3"
  },
  "devDependencies": {
    "grunt": "~1.0",
    "grunt-browserify": "~5.0",
    "grunt-cli": "~1.2",
    "grunt-contrib-concat": "~1.0",
    "grunt-contrib-connect": "~1.0",
    "grunt-contrib-copy": "~1.0",
    "grunt-contrib-cssmin": "~1.0",
    "grunt-contrib-jasmine": "~1.0",
    "grunt-contrib-jshint": "~1.0",
    "grunt-contrib-uglify": "~1.0",
    "grunt-contrib-watch": "~1.0",
    "grunt-docco2": "~0.2",
    "grunt-fileindex": "~0.1",
    "grunt-gh-pages": "~1.1",
    "grunt-jscs": "~2.8",
    "grunt-jsdoc": "~2.0",
    "grunt-jsdoc-to-markdown": "~1.2",
    "grunt-lib-phantomjs": "~1.1",
    "grunt-markdown": "~0.7",
    "grunt-saucelabs": "~8.6",
    "grunt-shell": "~1.3",
    "grunt-template-jasmine-istanbul": "~0.4",
    "ink-docstrap": "~1.1",
    "jsdifflib": "~1.1",
    "load-grunt-tasks": "~3.5",
    "marked": "~0.3",
    "time-grunt": "~1.3",
    "uglify-js": "~2.6"
  }

I also want to point out that, bower is not using crossfilter2. Is that correct?

@gordonwoodhull
Copy link
Contributor

Good points.

Emu was part of the old doc-chain, when we were pulling markdown directly from comments.

I'll update bower.json - another oversight. Too many package systems.

@gordonwoodhull
Copy link
Contributor

Um, I'm no expert, but I think you need to test npm dependencies on a fresh directory, i.e.
rm -rf node_modules

I'm getting a bucket of hate on grunt@1:

npm ERR! peerinvalid The package grunt@1.0.1 does not satisfy its siblings' peerDependencies requirements!
npm ERR! peerinvalid Peer grunt-fileindex@0.1.0 wants grunt@~0.4.1
npm ERR! peerinvalid Peer grunt-contrib-concat@1.0.1 wants grunt@>=0.4.0
npm ERR! peerinvalid Peer grunt-shell@1.3.0 wants grunt@>=0.4.0
npm ERR! peerinvalid Peer grunt-gh-pages@1.1.0 wants grunt@>=0.4.0
npm ERR! peerinvalid Peer grunt-contrib-connect@1.0.2 wants grunt@>=0.4.0
npm ERR! peerinvalid Peer grunt-jsdoc@2.0.0 wants grunt@>=0.4.5
npm ERR! peerinvalid Peer load-grunt-tasks@3.5.0 wants grunt@>=0.4.0
npm ERR! peerinvalid Peer grunt-saucelabs@8.6.2 wants grunt@~0.4.1
npm ERR! peerinvalid Peer grunt-contrib-jshint@1.0.0 wants grunt@>=0.4.0
npm ERR! peerinvalid Peer grunt-template-jasmine-istanbul@0.4.0 wants grunt@>=0.4.0
npm ERR! peerinvalid Peer grunt-docco2@0.2.1 wants grunt@~0.4
npm ERR! peerinvalid Peer grunt-jsdoc-to-markdown@1.2.1 wants grunt@>=0.4.0

@gordonwoodhull
Copy link
Contributor

Backing out to "grunt": "~0.4" seems to work.

@gordonwoodhull
Copy link
Contributor

gordonwoodhull commented May 23, 2016

Also grunt-contrib-jshint@1 spews a bunch of (to me) nonsensical errors about function definitions using those same functions before they were defined. So I'm backing that one out as well.

...
   src/row-chart.js
    132 |    function drawChart () {
                      ^ 'drawChart' was used before it was defined.
    146 |    function createElements (rows) {
                      ^ 'createElements' was used before it was defined.
    159 |    function removeElements (rows) {
                      ^ 'removeElements' was used before it was defined.
    168 |    function updateElements (rows) {
                      ^ 'updateElements' was used before it was defined.
    206 |    function createTitles (rows) {
                      ^ 'createTitles' was used before it was defined.
...
>> 139 errors in 67 files

@mtraynham
Copy link
Contributor

mtraynham commented May 23, 2016

Yeah, I mentioned the "latedef": false jshint one above. Not much changed between grunt 0.4 and 1.0, a lot of people just haven't migrated yet. NPM peer dependencies have been deprecated so I always just overlook that error, but feel free to leave it at 0.4

@gordonwoodhull
Copy link
Contributor

gordonwoodhull commented May 23, 2016

The latedef problem looks totally spurious, though.

Usually this is a useful check - but it's labelling declarations as uses and I think that must be their bug. So I'd rather not disable it until I find out what's going on. Couldn't find anything in a quick search.

@gordonwoodhull
Copy link
Contributor

Hrrrm, any npm-shrinkwrap.json seems to prevent npm from installing plain dependencies, i.e. d3 and crossfilter. Since grunt-copy (?) is happy skipping over files, I wouldn't have noticed, except that grunt-browserify fails.

I have to put this down for today. Pushing to hotfix/2.0.0-beta.30 branch in case anyone wants to tell me what I'm doing wrong here. (I did try devDependencies in npm-shrinkwrap.json as well.)

@gordonwoodhull
Copy link
Contributor

gordonwoodhull commented May 24, 2016

Okay, I had to explicitly shrinkwrap those two as well (in 3eaab78). This is not a great solution and I'll probably fork grunt-gh-pages if the maintainers don't respond. But I think the build is back in operation.

@gordonwoodhull
Copy link
Contributor

Nice, @jaridmargolin has done a PR, so we can probably switch to his fork until this gets merged:
tschaub/grunt-gh-pages#62

gordonwoodhull added a commit that referenced this issue Oct 6, 2016
yay a real fix for #1142
@gordonwoodhull
Copy link
Contributor

.. and that's merged, so we have proper fix!

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

3 participants