Replies: 1 comment
-
Just to mention, master from 9.28 to 9.28.2 is broken on partials and layouts. If you get a |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
Hi guys, thank you for stick with LiquidJS. We recently released v9.28 with streaming and relative includes, but it exposes multiple bugs (mainly on
root
option,fs
option,render/layout/include
tags) at the same time. After some investigation I find it difficult to keep all the promised features in #412 #419 #395 without breaking or non-intuitive change. So here's a summary of these features/bugs and what I'm planning to do:Current State Summary
Features we need/want to implement:
{% include "../paritals/footer.liquid %}
{% include "../../../../../etc/passwd" %}
partials
andlayouts
, apart from existingroot
optionroot
passed tofs.resolve(root, file, ext)
Problems:
file
is withinroot
, currently there's no such capability onfs
interface/optionOptions and plan
Option1: add an optional
fs.contains
(Maybe I'm planning to do this)contains
method in FileSystem.Option2: use existing
fs.resolve
to normalizeroot
,partials
,layout
option, add afs.sep
option.fs
interface is more clean and more intuitive.file
now could beundefiend
infs.resolve(root: string, file: string, ext: string)
.fs.resolve(root: string, ...)
will be a normalized/resolved root instead of theroot
option specified when creating Liquid instance.contains
logic implementing Access Control will be hardcoded asfile.startsWith(root + fs.sep)
, maybe confusing for someone come up with a filesystem implementation not aligh with this assumption.What you think? I need to know what's your opinion. Please feel free to post your ideas.
Beta Was this translation helpful? Give feedback.
All reactions