-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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 x-ua-compatible of null #1182
Conversation
up 👍 or maybe this should be fix in nodejs repo ? |
Hi! Thanks for PR. Try reproduce on node 0.13. Before node returned empty object if no query string and second argument was |
@@ -37,7 +37,7 @@ var filePathToUrlPath = function(filePath, basePath) { | |||
var getXUACompatibleMetaElement = function(url) { | |||
var tag = ''; | |||
var urlObj = urlparse(url, true); | |||
if (urlObj.query['x-ua-compatible']) { | |||
if (urlObj.query && urlObj.query['x-ua-compatible']) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can remove duplicate check in two functions(Later if they will return empty object then we change only one place) decorated urlparse
:
var url = require('url');
var urlparse = function(url){
var urlObj = url.parse(url, true);
urlObj.query = urlObj.query || {};
return urlObj;
};
Then our code will look:
var urlObj = urlparse(url);
if (urlObj.query['x-ua-compatible']) {
...
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, i think it makes more sens to fix/patch the urlparse function instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will fail, url in the function will be the passed string now
var url = require('url');
var urlparse = function(url){
var urlObj = url.parse(url, true);
urlObj.query = urlObj.query || {};
return urlObj;
};
ill add this to my fork and commit the changes
var url = require('url');
var urlparse = function(urlStr) {
var urlObj = url.parse(urlStr, true);
urlObj.query = urlObj.query || {};
return urlObj;
}
LGTM Forgot ... Can you:
Thanks! 👍 |
Just for documentation purposes; I experience the issue on node |
I confirm: doesn't break Probably way more people will experience this issue now that |
@mzgol very true, It just showed up in my Travis builds. |
Cannot read property x-ua-compatible of null
Apparently karma currently does not work in latest version of Node. Read more here: karma-runner/karma#1182 Fixes #90
As a bugfix until karma-runner/karma#1182 is released with karma v0.12.14.
The latest version of Node.js has a bug that affects the Karma test runner. A patch has been merged to Karma, but has not landed in a version yet. Until a new version of Karma is released, we should keep node at 0.11.13. See karma-runner/karma#1182 (cherry picked from commit 9af2346a0cead634f3af5f390770ea65929c1f4a)
The previous version is incompatible with Node 0.11.14, see: karma-runner/karma#1182
Some of previous dependencies versions (e.g. Karma) didn't work with Node 0.11.14, see: karma-runner/karma#1182 The only dependency not updated in this commit is grunt-jscs-checker; its rules have changed a lot so it will require more work to use the newer version.
Some of previous dependencies versions (e.g. Karma) didn't work with Node 0.11.14, see: karma-runner/karma#1182 The only dependency not updated in this commit are: 1. grunt-jscs-checker: its rules have changed a lot so it will require more work to use the newer version 2. gulp-jshint: the update breaks docs linting, it requires investigation
Some of previous dependencies versions (e.g. Karma) didn't work with Node 0.11.14, see: karma-runner/karma#1182 The only dependency not updated in this commit are: 1. grunt-jscs-checker: its rules have changed a lot so it will require more work to use the newer version 2. gulp-jshint: the update breaks docs linting, it requires investigation Closes angular#9571
Some of previous dependencies versions (e.g. Karma) didn't work with Node 0.11.14, see: karma-runner/karma#1182 The only dependencies not updated in this commit are: 1. grunt-jscs-checker: its rules have changed a lot so it will require more work to use the newer version 2. gulp-jshint: the update breaks docs linting, it requires investigation Closes angular#9571
Some of previous dependencies versions (e.g. Karma) didn't work with Node 0.11.14, see: karma-runner/karma#1182 The only dependencies not updated in this commit are: 1. grunt-jscs-checker: its rules have changed a lot so it will require more work to use the newer version 2. gulp-jshint: the update breaks docs linting, it requires investigation Closes angular#9571
Some of previous dependencies versions (e.g. Karma) didn't work with Node 0.11.14, see: karma-runner/karma#1182 The only dependencies not updated in this commit are: 1. grunt-jscs-checker: its rules have changed a lot so it will require more work to use the newer version 2. gulp-jshint: the update breaks docs linting, it requires investigation Closes #9571
The latest version of Node.js has a bug that affects the Karma test runner. A patch has been merged to Karma, but has not landed in a version yet. Until a new version of Karma is released, we should keep node at 0.11.13. See karma-runner/karma#1182 (cherry picked from commit 9af2346a0cead634f3af5f390770ea65929c1f4a)
The latest version of Node.js has a bug that affects the Karma test runner. A patch has been merged to Karma, but has not landed in a version yet. Until a new version of Karma is released, we should keep node at 0.11.13. See karma-runner/karma#1182 (cherry picked from commit 9af2346a0cead634f3af5f390770ea65929c1f4a)
Apparently karma currently does not work in latest version of Node. Read more here: karma-runner/karma#1182 Fixes #90
node v0.13.0-pre url.parse query returns null if no query string, and getXUACompatibleUrl and getXUACompatibleMetaElement are not checking for this