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

Empty document #195

Closed
stevenvachon opened this issue May 15, 2017 · 10 comments
Closed

Empty document #195

stevenvachon opened this issue May 15, 2017 · 10 comments
Labels

Comments

@stevenvachon
Copy link
Contributor

stevenvachon commented May 15, 2017

What are the situations in which parse5 would produce this with the streaming API?:

{ nodeName: '#document', mode: 'no-quirks', childNodes: [] }

Even an empty document at least produces html/head/body elements in my test suite. I'm getting the above with manual testing with a remote server.

@inikulin
Copy link
Owner

inikulin commented May 15, 2017

parse5 produces such AST if write or end was never called on a stream. Basically, if stream is in initial state and haven't received any data yet.

@stevenvachon
Copy link
Contributor Author

stevenvachon commented May 15, 2017

Hmm, "never called"? Am I supposed to wait for an event before sending the stream to parse5?

const parser = new parse5.ParserStream(options)
  .once("finish", () => resolve(parser.document));

input.pipe(parser);

If nothing was received, why was "finished" emitted? I'm confused.

@inikulin
Copy link
Owner

Interesting, seems like node stream doesn't call _write on end without content. Well, that's a bug, as a fix we can write empty string to stream on construction. Care to issue a PR?

@inikulin inikulin reopened this May 15, 2017
@inikulin inikulin added bug and removed question labels May 15, 2017
@stevenvachon
Copy link
Contributor Author

It might not be a bug. But you still may want to take some sort of action on this. I was accidentally using bhttp's discardResponse option for non-HEAD requests; which pipes to /dev/null. Do you still want to handle this with an empty string for consistent output, or is this a situation where the developer fucked up and deserves their fate?

@inikulin
Copy link
Owner

It's a bug. We give erroneous AST on empty string input (which is still a valid input)

@stevenvachon
Copy link
Contributor Author

stevenvachon commented May 15, 2017

I have tests with empty string input that correctly produce ASTs with html/head/body elements.

@inikulin
Copy link
Owner

Try

const parser = new parse5.ParserStream(options)
  .once("finish", () => resolve(parser.document));

parser.end();

@stevenvachon
Copy link
Contributor Author

stevenvachon commented May 15, 2017

That's different than an empty string, though.

@inikulin
Copy link
Owner

@stevenvachon That's pretty much the same.

@stevenvachon
Copy link
Contributor Author

stevenvachon commented May 15, 2017

I just noticed that my project's empty string tests were using parse5.parse() and not ParserStream. Testing ParserStream with an empty file or a file containing only whitespace results in the same code as in the initial comment above.

Also, which file do you want the test in?

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

No branches or pull requests

2 participants