fastify / point-of-view

Template rendering plugin for Fastify
MIT License
338 stars 86 forks source link

feat: consider multiple root paths for liquidjs engine #294 #295

Closed devjayantmalik closed 2 years ago

devjayantmalik commented 2 years ago

Checklist

devjayantmalik commented 2 years ago

I have added another test case as suggested by @darkgl0w

devjayantmalik commented 2 years ago

I think we should get rid of this functionality completely and care about it in future, because currently nodejs doesn't support async file checks or we should find some better alternative.

Moreover I think the module is quite unstable, I will work on this module in future, whenever I get time. Till then Work it out guys!

Here is how:

Also, there is a issue with point-of-view package, it doesn't consider liquidJS options root path, rather it works, only if you provide root path to POV options.

Here is what I mean:

    const engine = new Liquid({
      root: "./templates",
    });
    fastifyServer.register(POV, {
      engine: { liquid: engine },
    });

// Doesn't work because root is still ./
res.view("hello.liquid");

// It works because root dir inside package is still ./ it is not overwritten by liquid root path.
res.view("./templates/hello.liquid");

Here is the workaround solution to above problem:

// It works because root is ./templates res.view("hello.liquid");

// It doesn't work because root dir inside package is still ./templates and it becomes: ./templates/templates/hello.liquid res.view("./templates/hello.liquid");

mcollina commented 2 years ago

I think we should get rid of this functionality completely and care about it in future,

What functionality? Should we close this PR?

because currently nodejs doesn't support async file checks or we should find some better alternative.

I don't understand this.

Eomm commented 2 years ago

because currently nodejs doesn't support async file checks or we should find some better alternative.

You can use fs.access() to check it.

I hope your statements do not come as a reaction to my PR review 😞

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.