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

Upgrade Liquid major version #469

Closed
zachleat opened this issue Mar 29, 2019 · 12 comments
Closed

Upgrade Liquid major version #469

zachleat opened this issue Mar 29, 2019 · 12 comments
Labels
breaking-change This will have to be included with a major version as it breaks backwards compatibility.

Comments

@zachleat
Copy link
Member

We are on v6, they are on v8.

https://github.com/harttle/liquidjs/blob/master/CHANGELOG.md

Breaking changes apply to options name changes (no template facing changes here).

See also harttle/liquidjs#109

@zachleat zachleat added this to the Next Major Version milestone Mar 29, 2019
@paulrobertlloyd
Copy link
Contributor

Any reason for this not being in the v0.8.0 beta? I mis-read the release notes, and thought it had bumped Liquid to the latest major version, but was actually a minor one. I only ask as I came across an issue, and having certainty about the version used by 11ty (having tried to install the latest major version separately) would help me resolve it.

@zachleat
Copy link
Member Author

zachleat commented Apr 4, 2019

The reason is that it would break compatibility if anyone is using the options keys that were renamed. See https://www.11ty.io/docs/languages/liquid/#optional%3A-use-your-own-options

@zachleat
Copy link
Member Author

zachleat commented Apr 4, 2019

Hmm It’s possible we could shim those—I’ll have to think about it.

@kleinfreund
Copy link
Contributor

@zachleat I think this should just be scheduled with the next major version without shimming it for now.

#421 is also a breaking change and the follow-up PR to that will be, too. So releasing a major release could be valuable soonish?

@paulrobertlloyd
Copy link
Contributor

@zachleat Investigating using my own instance of Liquid, thought it was worth recording the raw build times seen when swapping out the built in version for the latest release.

With builtin LiquidJS instance (6.4.3), 5 runs:

  • Copied 4 items / Wrote 1463 files in 15.73 seconds (10.8ms each, v0.10.0)
  • Copied 4 items / Wrote 1463 files in 16.11 seconds (11.0ms each, v0.10.0)
  • Copied 4 items / Wrote 1463 files in 15.66 seconds (10.7ms each, v0.10.0)
  • Copied 4 items / Wrote 1463 files in 16.02 seconds (11.0ms each, v0.10.0)
  • Copied 4 items / Wrote 1463 files in 15.80 seconds (10.8ms each, v0.10.0)

Average build time: 15.86 seconds

With own LiquidJS instance (9.6.2), 5 runs:

  • Copied 4 items / Wrote 1463 files in 11.82 seconds (8.1ms each, v0.10.0)
  • Copied 4 items / Wrote 1463 files in 12.06 seconds (8.2ms each, v0.10.0)
  • Copied 4 items / Wrote 1463 files in 11.72 seconds (8.0ms each, v0.10.0)
  • Copied 4 items / Wrote 1463 files in 12.01 seconds (8.2ms each, v0.10.0)
  • Copied 4 items / Wrote 1463 files in 11.82 seconds (8.1ms each, v0.10.0)

Average build time: 11.89 seconds (~25% faster)

Seems like some sizeable performance gains to be had for Liquid-based sites!

@proog
Copy link

proog commented Feb 6, 2020

I will add that v6 is fairly incomplete compared to the official Liquid implementation, with several filters missing (that have since been added to liquidjs) such as where and sort_natural. I just spent the good part of an hour trying to figure out why the latter wasn't working in 11ty, and I feel the incomplete nature of the provided version should at least be made clear in the docs on Liquid templates.

@cfjedimaster
Copy link

As an FYI (and I know GitHub linked it already), I raised this issue at a high level here: #906.

@pdehaan
Copy link
Contributor

pdehaan commented Feb 20, 2020

+1 to shimming "strict_filters" to "strictFilters" when bumping up to liquidjs v9 (so that both are supported and we don't break anything).

@zachleat zachleat added this to Needs triage in Eleventy 1.0 via automation Feb 29, 2020
@zachleat zachleat added breaking-change This will have to be included with a major version as it breaks backwards compatibility. and removed housekeeping labels Mar 20, 2020
@MikeRalphson
Copy link

Lack of sort: "key" (fixed in liquidjs 9.12.0) makes liquid templates a complete no-go for us at the moment.

@zachleat
Copy link
Member Author

zachleat commented Sep 4, 2020

This shipped on master and will be included with 1.0. @pdehaan I filed your request at #1390

@zachleat zachleat closed this as completed Sep 4, 2020
Eleventy 1.0 automation moved this from Needs triage to Closed Sep 4, 2020
@paulrobertlloyd
Copy link
Contributor

🎉

@zachleat
Copy link
Member Author

zachleat commented Sep 4, 2020

I’ve updated the dependency again to the latest since the PR #1058 was a little old 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change This will have to be included with a major version as it breaks backwards compatibility.
Projects
No open projects
Eleventy 1.0
  
Closed
Development

No branches or pull requests

7 participants