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

[Feature] location property in AST #44

Closed
anikethsaha opened this issue May 6, 2020 · 2 comments · Fixed by #63
Closed

[Feature] location property in AST #44

anikethsaha opened this issue May 6, 2020 · 2 comments · Fixed by #63

Comments

@anikethsaha
Copy link
Member

currently, we don't process any location property for the nodes. It is because htmlparser2 doesn't support it from the core. I have seen some implementation where they are using domhandler to get the raw HTML and add location property in it. The implementation slightly went over my head and I do think it might break our current cast.

As discussed with @Scrum , we need to take a performance factor as well and we should not break implement anything that changes the AST. (we can add it. )

I am trying to implement at least the line property with the current htmlparser2 options but i haven't figure out the way to implement the columns. (still thinking. 🤔 )

I would appreciate a lot if anyone else has better approach without changing much.

I will submit a PoC soon with my changes.

cc @Scrum @cossssmin , thoughts ?

@devongovett
Copy link
Contributor

Did you make any progress on this? We need location information to provide nice error messages in Parcel. Happy to help out here if we can!

@Scrum
Copy link
Member

Scrum commented Nov 25, 2020

I will think how to do it optional

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

Successfully merging a pull request may close this issue.

3 participants