twigjs / twig.js

JS implementation of the Twig Templating Language
BSD 2-Clause "Simplified" License
1.88k stars 274 forks source link

Includes in Twig files throwing errors #808

Closed AtlantaJones closed 10 months ago

AtlantaJones commented 2 years ago

I'm using Twig.js in Storybook and at first it seemed to be working fine... until I tried to do an include in a twig file. I can't get it to work no matter what I try, and it's always throwing console errors related to fs and path.

For context, I'm using:

I've put together a very basic test, based on what's on the Twig.js Readme (note that test.twig is in the same folder as the test script):

import Twig from 'twig';

const test1 = Twig.twig({
  data: 'The {{ baked_good }} is a lie.',
}).render({ baked_good: 'cupcake' });

const test2 = Twig.twig({
  data: "The {{ baked_good }} is a lie. {% include 'test.twig' %}",
  allowInlineIncludes: true,
}).render({ baked_good: 'cupcake' });

console.log(test1, test2);

The first test works just fine, outputting "The cupcake is a lie.". However, test2 fails, with this series of messages:

I thought maybe it was a path issue, but adding the param path: './' just threw a new error: path.normalize is not a function

I've put together a Codepen with this sample code and although it can't load an included twig file, you can still see it throwing the fs.statSync is not a function error in the console.

So far I've spent 12 hours trying to figure out what is happening here. No matter what I do, a simple "include" in a twig file breaks it. I'm also following the instructions suggested by the Twig loader for Vite and again, things are fine until I try to include another twig file.

I've Googled for hours and can't find anyone having this exact problem, so I'm hoping I'm just missing something incredibly basic.

willrowe commented 2 years ago

It's interesting that you mention Storybook, there was just another issue that involved Storybook as well. Can you take a look through #806 and see if there is any potential that this is being caused by the same or a related issue?

It sounds to me like an issue with the loader configuration though. The Codepen you sent will only work inline if you provide the test.twig template inline as well.

import Twig from "https://cdn.skypack.dev/twig@1.15.4";

const test1 = Twig.twig({
  data: 'The {{ baked_good }} is a lie.',
}).render({ baked_good: 'cupcake' });

Twig.twig({
  'data': 'contents of test.twig',
  'id': 'test.twig',
})
const test2 = Twig.twig({
  data: "The {{ baked_good }} is a lie. {% include 'test.twig' %}",
  allowInlineIncludes: true,
}).render({ baked_good: 'cupcake' });

console.log(test1, test2);
AtlantaJones commented 2 years ago

Sorry for the delay, on vacation. I don't think #806 is the same issue, as it appears the includes work, they just aren't working with concatenation.

willrowe commented 2 years ago

Are you able to provide a simplified Codepen that reproduces the same errors you are seeing?

AtlantaJones commented 2 years ago

I'm not sure, unless it's possible to load an included twig file via an external URL. I can try. But I would think that since it's throwing fs.statSync is not a function on Codepen, the behavior is the same whether the twig file is there or not.

willrowe commented 2 years ago

If you're only having the issue when loading from external files, it sounds to me like a loader or configuration issue. Did you reference the Browser Usage and AJAX Templates sections in the docs?

AtlantaJones commented 2 years ago

Yes, but it doesn't seem to be anything different than what I've done. For now, I've had to scrap the integration with Vite and go back to our previous setup for Storybook using Webpack and twigjs-loader. At some point, we'll try to get it back to using Vite, as that's what the rest of our site's frontend uses. Was just hoping something in my original post would reveal an obvious red flag.

AtlantaJones commented 2 years ago

I could also set up a new Github repo that has the exact setup we were trying to use, if that would be something you'd be interested in seeing.

willrowe commented 2 years ago

I could also set up a new Github repo that has the exact setup we were trying to use, if that would be something you'd be interested in seeing.

Sure, that would be helpful. If it's working with Webpack, but not Vite, it sounds like a loader issue for sure. Having a repo to reference would allow me to determine if the loader isn't set up correctly or if there is a bug on the twig.js end.

AtlantaJones commented 2 years ago

Awesome, I'll work on something over the next day or so. Thanks for the time so far.

willrowe commented 2 years ago

Great, I'll keep an eye out for it. Thanks for being willing to provide the additional info in order to hopefully get to the bottom of what's going on.

mattstein commented 11 months ago

Did either of you happen to get anywhere with this?

I’m taking my second run at Storybook + Twig + Vite after retreating to webpack, and I first suspected my project structure was the problem but this tiny little reference repo exhibits the problem with very few pieces: https://github.com/mattstein/storybook-vite

It’s a barebones Vite project with Storybook HTML and vite-plugin-twig-loader, where I stripped out some extra cruft after confirming it had no impact.

I’ve hacked at the loader in a separate project, and I always get errors in one of two flavors:

  1. The undefined is not an object (evaluating 'output.valueOf') you’ll see in this test case.
  2. An “Error parsing twig template” with the wrong path, trying to incorrectly load /path/to/project/_components/_components/included.twig when it should be /path/to/project/_components/included.twig—which if I can fix will result in error 1. (I think this is just my struggle to set a Twig.js template root like I’m used to in another environment, so it could be a separate issue.)
calebsmithdev commented 11 months ago

Also following up on this thread to see if anyone has found a solution. I'm running into similar issues as well and I am guessing it's more-so related to how the loader package is working.

EDIT: We ended up just making the move to Twing. I saw another ticket that confirmed it would be the better approach for our use case, which is a WP theme.

willrowe commented 10 months ago

@mattstein I took a look at the repo you linked to, but it will not even build due to missing files. Can you please clone it from scratch and then fix the issues so I can test it out?

mattstein commented 10 months ago

@willrowe Hmm, I just cloned it on a different machine, ran npm install (with Node.js v18), and I can use both npm run storybook and npm run storybook-build as expected. Also ran serve storybook-static and made sure the built Storybook project works fine in a browser—which it does, with the exception of “Exhibit B” which demonstrates the problem.

I do see warnings running the storybook-build script, but the build finishes successfully:

vite v4.4.9 building for production...

./sb-common-assets/fonts.css doesn't exist at build time, it will remain unchanged to be resolved at runtime
[plugin:vite:resolve] Module "path" has been externalized for browser compatibility, imported by "/path/to/storybook-vite/node_modules/twig/src/twig.loader.fs.js". See http://vitejs.dev/guide/troubleshooting.html#module-externalized-for-browser-compatibility for more details.
[plugin:vite:resolve] Module "path" has been externalized for browser compatibility, imported by "/path/to/storybook-vite/node_modules/twig/src/twig.path.js". See http://vitejs.dev/guide/troubleshooting.html#module-externalized-for-browser-compatibility for more details.
node_modules/telejson/dist/index.mjs (1413:15) Use of eval in "node_modules/telejson/dist/index.mjs" is strongly discouraged as it poses security risks and may cause issues with minification.
node_modules/telejson/dist/index.mjs (1416:18) Use of eval in "node_modules/telejson/dist/index.mjs" is strongly discouraged as it poses security risks and may cause issues with minification.

...

✓ built in 3.05s
info => Output directory: /Users/mattstein/Projects/storybook-vite/storybook-static

Still happy to try and fix whatever issues you’re seeing or reproduce elsewhere if it helps!

willrowe commented 10 months ago

@mattstein ok, I was running npm run build and npm run dev instead of the storybook scripts.

After taking a look, I think there has been a misunderstanding here about how twig.js works. Storybook is running in a browser, so it does not have access to the node fs and path libraries. However, due to the Vite loader, it is attempting to use the fs template loader. The included template files either needed to be added to the template registry when building the files so they are available in memory or a XHR/fetch template loader needs to be used instead. The only issue I see here from the twig.js end of things is that the error messages are being swallowed in some cases and allowing rendering to continue even when it should not. This is a known issue and is being tracked elsewhere.

I am going to close this since it appears to be a configuration issue somewhere in the Vite loader and is outside the scope of twig.js itself.

mattstein commented 10 months ago

I should’ve clarified that to start with—thanks for taking a look and explaining that, @willrowe! Sounds like my next step should be to explore the webpack loader and see how it preloads and references templates under the hood.

ericmorand commented 10 months ago

The main issue is that storybook compiles things in the browser. This has been an issue from the beginning and is likely to remain one in the future: this is a design decision that is enough a bad one to prevent a lot of us from using it and use our own build tool chain that compile things in the server.

mattstein commented 10 months ago

@ericmorand I’ve been looking at @storybook/server with mixed feelings since it’d add a persistent external dependency. I’d almost rather lean more heavily into duplicating Twig environments with Twig.js to keep a fully-static reference.

MariaKern commented 9 months ago

I should’ve clarified that to start with—thanks for taking a look and explaining that, @willrowe! Sounds like my next step should be to explore the webpack loader and see how it preloads and references templates under the hood.

Hi @mattstein did you already looked into the webpack loader and found a solution for the vite-plugin-twig-loader? I have the same issue here and this would be glad! Both suggestions from @willrowe would mean adjustments to the plugin, I think.

mattstein commented 9 months ago

Not yet, @MariaKern. If make any kind of progress I’ll follow up here though!

IHIutch commented 9 months ago

It looks like there is a psuedo fork of vite-plugin-twig-loader here: vite-plugin-twig-drupal, trying to get includes/embeds/extends working. Wonder if it might help the issues you're experiencing. Although I haven't been able to get Macros working: https://github.com/larowlan/vite-plugin-twig-drupal/issues/5

larowlan commented 9 months ago

FWIW, its a fresh rewrite rather than a fork. But I did try the other loader first.

mattstein commented 9 months ago

@larowlan’s Drupal loader works great—switched to it and updated my story format and all’s well!

IHIutch commented 5 months ago

Coming back to share a workaround I recently discovered.

Looks like when using Vite, and in this case Storybook, (in the example its Storybook 8 and Vite 5, but Im not sure that matters) you can import the relative asset path (./path/to/your/import?url) using Explicit URL Imports and pass that to Twig.twig as the href parameter.

It seems by providing the href, the twig templates use the ajax method rather than the fs method, as mentioned above, which avoids the fs issues originally mentioned.

So, a simple example:

import { twig } from "twig"
import twigTemplate from "./path/to/your/twig_template.twig?url"
import twigVariables from "./path/to/your/twig_variables.json"

const renderedHTML =  twig({
    async: false, // required
    href: twigTemplate,
  }).render(twigVariables)

In the provided example I'm using some imports and macros and seems to work well!

ericmorand commented 5 months ago

For information, twing-loader - the webpack loader that uses Twing under the hood - has been supporting macros, imports, extends, embeds (basically the whole Twig specifications) for years. The next version 5.x - fueled by Twing 6 and thus supporting the entire Twig 3 specifications - is due in the next few hours.

It will allow writing @IHIutch code this way - without any workaround using ?url or href:

import { createEnvironment, createArrayLoader } from "twing"
import twigTemplate from "./path/to/your/twig_template.twig"
import twigVariables from "./path/to/your/twig_variables.json"

const environment = createEnvironment(createArrayLoader({}));

const renderedHTMLPromise = twigTemplate.render(environment, twigVariables);

Some interesting things to note:

import {createEnvironment, createFilesystemLoader} from "twing";
import fs from "fs";

module.exports = {
    entry: 'index.js',
    // ...
    module: {
        rules: [
            {
                test: /\.twig$/,
                use: [
                    {
                        loader: 'twing-loader',
                        options: {
                            environment: createEnvironment(createFilesystemLoader(fs))
                        }
                    }
                ]
            }
        ]
    }
}

EDIT: If you want a peek on the upcoming 5.0.0 version of the loader, here is the development branch: https://gitlab.com/nightlycommit/twing-loader/-/tree/milestone/5.0.0/src

willrowe commented 5 months ago

@ericmorand I don't mind you contributing insights from building your package that could help with the development and support of twig.js, but please refrain from promoting it here. It is not very helpful for those who are seeking information about twig.js specifically.

ericmorand commented 5 months ago

You are right. I wrongly assumed that Vite plugins depended directly on Webpack loaders and, thus, that the Twing webpack loader could be of some help here. I'll hide my comment to not create any further confusion. Sorry about the noise.