refactor: Remove liquid templates
I noticed when working on #352 that the Liquid decency is outdated and flagged in a security issue. This PR refactors the preview site templates to use a simpler approach as we're not using any of the features Liquid provides.
Thanks @abovedave for the PR. I am all up for simplifying but consider that we have recently introduced liquid templates for mocking Core's layout in new Free Refills services. I don't see our usage of liquid going away any time soon.
Perhaps there is another webpack loader we can use instead of removing the liquid templates all together? Where can I read more about the vulnerabilities of that loader?
Thank you
Ah I see! I could just swap out the new Function() with the actual Liquid package and skip the unmaintained loader if we really want it? It just seemed uncessary to use liquid just for a simple layout-view setup.
This was the error:
# npm audit report
liquidjs <10.0.0
Severity: moderate
liquidjs may leak properties of a prototype - https://github.com/advisories/GHSA-45rm-2893-5f49
No fix available
node_modules/liquidjs
liquidjs-loader *
Depends on vulnerable versions of liquidjs
node_modules/liquidjs-loader
It just seemed uncessary to use liquid just for a simple layout-view set
@abovedave I would like to talk to Dan before removing Liquid all together from this repo. If the new Function() approach can buy us some time and fix the vulnerability I would prefer it. Thanks 🙏
Deploy Preview for stacks-editor ready!
| Name | Link |
|---|---|
| Latest commit | 68c162cb17613ea77f96dbfeff1475d531bd1538 |
| Latest deploy log | https://app.netlify.com/sites/stacks-editor/deploys/66fbe8db3a4930000889de51 |
| Deploy Preview | https://deploy-preview-353--stacks-editor.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
I haven't reviewed this PR yet (expect a review later today), but I'm 100% ok with dropping liquid in this repo. The reason I introduced it was because I wanted a simple shared layout for the docs and liquid syntax was already used in Stacks's docs. The use of liquid in this repo is probably overkill, but it worked ok and stayed out of the way. If it is causing issues now, then let's ditch it.
@b-kelly If we are going to keep a templating language of sort, I would prefer to stay consistent with what we use in Stacks docs. We have just started to use liquid also for some free refills mock layout stuff (see WIP). Perhaps it is just a matter of swapping the existing loader with one that does not have vulnerabilities. I haven't looked in details but maybe this one could be an option.
@giamir I'm ok with adding back in a special loader or templating language, but I think it's overkill for our use case. 100% of the code that does the "templating" in this case is just a single line:
htmlLayout.replace("<%=content%>", content)
So... we really don't need a full blown templating solution for this. Imo, we can revisit in the future if we ever need a more full-featured solution.