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

find templates with custom babel parser #175

Closed
wants to merge 2 commits into from

Conversation

patricklx
Copy link
Contributor

@patricklx patricklx commented Jul 6, 2023

i noticed that the current template parsing has issues.

I created a custom parser for babel which can parse the template tags.
https://babeljs.io/docs/babel-parser#will-the-babel-parser-support-a-plugin-system

I also added a conditional prefix to fix #171

@patricklx
Copy link
Contributor Author

patricklx commented Jul 6, 2023

Will try with htmlparser2. No need for full spec (parse5) compliance here. Better fast and forgiving :)

@patricklx patricklx marked this pull request as ready for review July 7, 2023 09:29
@patricklx patricklx force-pushed the use-parsers branch 3 times, most recently from 23d1429 to 7b2d14b Compare July 7, 2023 10:18
@patricklx
Copy link
Contributor Author

this is ready now

@patricklx patricklx force-pushed the use-parsers branch 6 times, most recently from 7b9734f to 933059d Compare July 10, 2023 13:06
@patricklx
Copy link
Contributor Author

patricklx commented Jul 15, 2023

@dfreeman @ef4 @chriskrycho what do you think?

@dfreeman
Copy link
Collaborator

I'm not an active maintainer here or on Embroider, but my guess is that https://github.com/embroider-build/content-tag is a more complete solution to the issues you're aiming to tackle here.

@ef4
Copy link
Collaborator

ef4 commented Jul 20, 2023

Yes, that is correct and we hope that content-tag is the much more robust solution. It's rust compiled to wasm, based on SWC so it has the benefit of a full javascript parser.

The intention is to PR changes to ember-template-imports that would use content-tag to replace the preprocessor stage, and the babel stage here will also not be needed at all because babel-plugin-ember-template-compilation has already shipped the corresponding feature that goes with content-tag.

@patricklx
Copy link
Contributor Author

patricklx commented Jul 20, 2023

What about other packages that depend on this to find the template parts? Like the eslint ember plugin. template lint etc.
Though i would be happy to make a pr for content-tag

@ef4
Copy link
Collaborator

ef4 commented Jul 20, 2023

Yes we want to standardize all those things on content-tag too. We can output whatever info from it is most convenient for those tools.

It's actually problematic right now that the preprocessor here lives in a classic ember addon. It's bad that things like template lint need to depend on this full addon, because the addon does more than just provide the preprocessor library.

@patricklx
Copy link
Contributor Author

So, it is already in a state that can be used for this?

@patricklx patricklx force-pushed the use-parsers branch 2 times, most recently from a5d44c6 to b9b7132 Compare July 21, 2023 12:32
@patricklx patricklx changed the title find templates with html parser and js parser find templates with custom babel parser Jul 21, 2023
@patricklx
Copy link
Contributor Author

content-tag does not look like its ready #182

@patricklx
Copy link
Contributor Author

also extracted this into: https://github.com/patricklx/ember-template-preprocessor

@ef4
Copy link
Collaborator

ef4 commented Jul 21, 2023

Thanks for starting this work, I was able to extract a reproduction of the bug you found. embroider-build/content-tag#16

@NullVoxPopuli
Copy link
Collaborator

gonna close this as

  • bulk of the work was extracted to another project
  • we have content-tag

thanks again for your hard work @patricklx, it's much appreciated!

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.

missing semicolon in class attributes breaks build
4 participants