frctl / twig

Use Twig templates with Fractal.
32 stars 34 forks source link

Include twig file from node_modules #42

Closed julkue closed 4 years ago

julkue commented 4 years ago

This is a follow-up issue from my comment here.

I would like to include a Twig file within the node_modules folder from a components Twig file. This doesn't work currently, because node_modules is located outside the configured components directory.

I've already experiemented with the base option and with Twig namespaces (#25), but unfortunately nothing worked. Base only seems to be a base within the components directory, so it's relative to it and can't be an absolute path. The same applies for Twig namespaces. Using the root directory of the project as component directory won't work, as it would also check all the node_modules whether they are components to show in the library – which takes years.

The reason why I need this? Because I'm building a shared frontend components library that we can re-use in all customer projects. You can import and directly export the fractal.js file of this components library that was included as dependency using npm successfully like described in the Fractal docs. But then you can't extend this library or just show some of the components.

Therefore I've decided to embed the shared components library as npm dependency and import JS/CSS/Twig files individually, so I have to re-create all components that I want to show in a customer project. For JS/CSS it's fine to import files from node_modules, but Twig is the show stopper currently.

mihkeleidast commented 4 years ago

Just to over the expected behavior: given that you have a twig file in your node_modules folder, for example in node_modules/shared-lib/component.twig, you'd like to include that template in the current project, for example like so:

{% include '~shared-lib/component.twig' %}

(I've prefixed the include with ~ since it's familiar to me from how sass imports from node_modules work with webpack. but it's totally up for debate what prefix would be best for this.)

Is that what's expected? This does not seem too complicated to implement, a second loader could be added similarly to what's going on here that should resolve to node_modules instead of to fractal api.

However, it's straightforward to me only when including a single file. What happens when that component.twig tries to include another component/twig template? How should those deeper includes be resolved?

mihkeleidast commented 4 years ago

Or... would the expected behavior to optionally bypass the fractal template loader entirely and use the default fs loader that ships with twig.js? Then it could be done by allowing the method option of twig.js be overriden via the adapter options here.

julkue commented 4 years ago

@risker Yes, something like this is what I need. However, I would prefer to use native Twig namespaces like e.g.

{% include '@SharedComponents/button/button.twig' %}

As when backender will try to integrate this, they will have it much more easier.

What happens when that component.twig tries to include another component/twig template?

In my case this will be the case. These components can include other component within the shared components library by using the same syntax like above. I could configure those includes to use the same namespace like mentioned above so it would map with the configuration of the project using the shared library.

In PHP Twig we can then configure exactly the same namespaces and we could simply re-use the same templates.

What happens when that component.twig tries to include another component/twig template?

I'm not sure whether this would allow includes outside the configured components library. Would this then allow imports at any place?

mihkeleidast commented 4 years ago

So regarding my second comment, do you need the fractal template loader at all? Does the twig.js default fs template loader work as expected for your use case?

You could test that out by setting the method to 'fs' here

julkue commented 4 years ago

@risker Unfortunately it doesn't seem to be that easy. Just replacing it with fs will cause an error when opening any component, because Fractal passes the handler to the engine and not the real path. So the error will look like:

TwigException: Unable to find template file @text-image-left-right--three-quarters-width

I also tried replacing name: meta.self ? `${self._config.handlePrefix}${meta.self.handle}` : tplPath, with name: self._source.fullPath, but that cuases a similar error (Unable to find template file) when just opening a component without any includes (Twig.js is unable to find the opened components template file itself). The same issue occurs when using name: tplPath,.

mihkeleidast commented 4 years ago

something i did is up in #43. the name option should only be used with a custom loader (i think) and not when using the default loader. and passing through the path option seemed to work for my simple tests with two or three components.

if using that branch with these options:

const twigAdapter = require('@frctl/twig')({
    method: 'fs',
});

my templates resolved correctly via relative paths. that of course means that including components via fractal handles is not available. do those changes make any progress in what we're trying to achieve?

julkue commented 4 years ago

@risker First of all, thank you so much for your efforts!

This works with relative paths 👍 Now, the interesting part is to combine this with namespaces (see e.g. #25).

Currently my import from node modules looks like:

{% include '../../../../node_modules/cando-shared-frontend-components-library/src/components/03-common/text-image-left-right/text-image-left-right.twig' %}

What I would like it to be is something like:

{% include '@SharedComponents/03-common/text-image-left-right/text-image-left-right.twig' %}

and for component library internal (non-shared-library) imports (this is already how I'm doing it currently with the branch from #25, instead of using relative paths):

{% include '@Components/03-common/text-image-left-right/text-image-left-right.twig' %}

I can achieve both with native PHP Twig. I just have to configure the namepsaces like e.g.

        '%kernel.project_dir%/../frontend/components-library/src/components': Components
        '%kernel.project_dir%/vendor/cando/shared-components-library/src/components': SharedComponents

Therefore I would then be able to even share the template with the backend directly.

julkue commented 4 years ago

I've just experimented with namespaces too. When merging the changes from #25 into your PR, then configuring it like:

fractal.components.engine(twigAdapter({
  method: 'fs',
  namespaces: {
    Components: path.join(__dirname, '../src/components'),
    SharedComponents: path.join(__dirname, '../node_modules/cando-shared-frontend-components-library/src/components')
  },
}));

Then I can even use namespaces successfully. This is awesome 👍

julkue commented 4 years ago

@risker I see the only disadvantage is that then this will no longer work:

{% render '@text-image-left-right' %}

causes:

TypeError: this.importFile is not a function

I previously used include in all my components with namespaces. However, I had a few templates where I collected some components (page templates) and I used render to avoid redundant context data for that.

But if this is the only downside (?) then I can live with that.

mihkeleidast commented 4 years ago

awesome that it worked! i'll finish up that PR then and we'll see if we can get those changes merged next week.

i don't see why the render tag should stop working really, i believe it does it's own lookups directly to the fractal api... the this.importFile is not a function error looks like it could be thrown from here but we changed it recently to fix compatibility with newer twig.js versions. could it be that you have the render tag source file out of date?

anyhow, i can take a look at it if the error's coming from elsewhere, i don't currently see why render tag should stop working.

mihkeleidast commented 4 years ago

investigated a bit. did not get the same error when trying to use render tag with the render tag source updated, but it still did not work. i guess it still tries to pass that template name to the loader, and since it's just a handle, it fails.

So yes, the downside to using the default twig.js fs template loader is that none of the fractal-specific functionality (including components via handles or the render tag) will work in the templates.