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

Make it work with node v6.0.0 #103

Closed
wants to merge 2 commits into from
Closed

Conversation

jackhsu978
Copy link
Contributor

I recently upgraded node to v6, but this module is broken after the upgrade. The error message is:

TypeError: parsedQuery.hasOwnProperty is not a function

To reproduce the problem, try evaluate the following with Node v6:

require('url').parse('', true).query.hasOwnProperty('test')

It turns out that when query is an empty string, the query object returned from url.parse has no method hasOwnProperty. This patch fixes the problem.

When query is an empty string, the query object returned from
`url.parse` does not inherit from Object and thus has no method
`hasOwnProperty`
@thoop
Copy link
Contributor

thoop commented Apr 28, 2016

Looks like no matter what, it evaluates to undefined:

url.parse('http://www.example.com?test=true', true).query.hasOwnProperty

And it looks like they changed the querystring module to call Object.create(null) to return an "empty" object instead of {}

I don't think _escaped_fragment_ would ever be on a parent prototype object so I think everything would be ok if we just shortened it to:

if(parsedQuery && parsedQuery['_escaped_fragment_']) isRequestingPrerenderedPage = true;

What do you think?

@jackhsu978
Copy link
Contributor Author

@thoop yes, I agree. That's actually better!

@thoop
Copy link
Contributor

thoop commented Apr 28, 2016

Want to send that in a new pull request and I'll merge it? Thanks for finding that!

@jackhsu978
Copy link
Contributor Author

Just sent another pull request 😄

@jackhsu978
Copy link
Contributor Author

See pull request #104

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

Successfully merging this pull request may close these issues.

None yet

2 participants