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

Improvements to Handlebars plugin #12037

Closed
wants to merge 13 commits into from

Conversation

j-f1
Copy link
Member

@j-f1 j-f1 commented Jan 2, 2022

Description

  • support for partials
  • support for decorators
  • support for partials inside attributes
  • support front matter in .hbs files

Note: for this PR to be possible, I had to make a bunch of changes to the parser (largely to remove restrictions, but also to correct some errors). I published these to npm as my own package since I get the impression that they would not be useful to the Glimmer project as a whole. Check them out here: https://github.com/j-f1/forked-glimmer-vm (compare with upstream)

I’m not 100% confident in all of the changes (but fortunately we only use small parts of the codebase, so perhaps it would be worth throwing out the rest of the code?) I would also like to have this fork managed/published under the Prettier org so I don’t wind up having to maintain it forever since I’m not sure how much time I can consistently commit.

Finally, I’ve given this PR a cursory check (by using it to format on save) on a relatively small Handlebars project, but it would be awesome to get people to try it out on larger codebases and see how well it works. Since the existing handlebars support is fairly limited, I wouldn’t be surprised if there are lots of changes to make formatting-wise.

Fixes #11834

Checklist

  • I’ve added tests to confirm my change works.
  • (If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory).
  • (If the change is user-facing) I’ve added my changes to changelog_unreleased/*/XXXX.md file following changelog_unreleased/TEMPLATE.md.
  • I’ve read the contributing guidelines.

Try the playground for this PR

@fisker
Copy link
Member

fisker commented Jan 2, 2022

I don't use handlebars myself, but is it possible to merge your work into @glimmer/syntax?

@j-f1
Copy link
Member Author

j-f1 commented Jan 2, 2022

I doubt it. Glimmer isn’t actually Handlebars; it’s a frontend framework that uses Handlebars syntax to enable React-style templating. That means that, among other things, they only support a subset of Handlebars features (bad for us) and that they need to be able to determine where the Handlebars template code should go in the HTML AST (good for us; this is what allows us to format both the Handlebars parts and The HTML parts). I could contact them to ask about upstreaming my changes, but I doubt they would want the technical debt of being able to parse things that they cannot support themselves.

@j-f1
Copy link
Member Author

j-f1 commented Jan 2, 2022

The Glimmer parser doesn’t actually handle parsing the HTML or the Handlebars parts; instead, its task is to merge the two ASTs into one.

@sosukesuzuki
Copy link
Member

Thank you for your great work!

However, I have concerns about maintainability about to fork parser library. There are probably no current active maintainers of Prettier who are familiar with Handlebars. So, it would be better to contact the upstream maintainer to merge your work.

@j-f1
Copy link
Member Author

j-f1 commented Jan 2, 2022

Opened glimmerjs/glimmer-vm#1367.

@j-f1
Copy link
Member Author

j-f1 commented Jan 4, 2022

Also added support for doctypes, fixing a bug I encountered: syntax-tree/hast-util-to-html#26

@j-f1
Copy link
Member Author

j-f1 commented Jan 24, 2022

I’ve migrated my own code from Handlebars to JSX, and there has been no response to the issue that was opened above. Closing this PR until somebody is either willing to maintain the forked parser or an alternate solution appears.

@j-f1 j-f1 closed this Jan 24, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SyntaxError: Handlebars partials are not supported
3 participants