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

Can't render child components when using twig for both components and docs #607

Closed
levito opened this issue Apr 17, 2020 · 5 comments · Fixed by #887
Closed

Can't render child components when using twig for both components and docs #607

levito opened this issue Apr 17, 2020 · 5 comments · Fixed by #887

Comments

@levito
Copy link
Contributor

levito commented Apr 17, 2020

When using twig for both components and docs (fractal.components.engine(twigAdapter) and fractal.docs.engine(twigAdapter)), I can't render child components anymore.

So I have components/a/a.twig, and components/b/b.twig with:

{% render "@a" %}

Visiting component B results in the following error:

Error: Error: Template @a not found
    at /path/to/node_modules/@frctl/twig/src/adapter.js:158:24
    at new Promise ()
    at TwigAdapter.render (/path/to/node_modules/@frctl/twig/src/adapter.js:140:16)
    at ComponentSource._renderVariant (/path/to/node_modules/@frctl/fractal/src/api/components/source.js:207:30)
    at _renderVariant.next ()
    at onFulfilled (/path/to/node_modules/co/index.js:65:19)
    at tryCatcher (/path/to/node_modules/bluebird/js/release/util.js:16:23)
    at Promise._settlePromiseFromHandler (/path/to/node_modules/bluebird/js/release/promise.js:547:31)
    at Promise._settlePromise (/path/to/node_modules/bluebird/js/release/promise.js:604:18)
    at Promise._settlePromise0 (/path/to/node_modules/bluebird/js/release/promise.js:649:10)
    at Promise._settlePromises (/path/to/node_modules/bluebird/js/release/promise.js:729:18)
    at Promise._fulfill (/path/to/node_modules/bluebird/js/release/promise.js:673:18)
    at Promise._resolveCallback (/path/to/node_modules/bluebird/js/release/promise.js:466:57)
    at Promise._settlePromiseFromHandler (/path/to/node_modules/bluebird/js/release/promise.js:559:17)
    at Promise._settlePromise (/path/to/node_modules/bluebird/js/release/promise.js:604:18)
    at Promise._settlePromise0 (/path/to/node_modules/bluebird/js/release/promise.js:649:10)
    at Promise._settlePromises (/path/to/node_modules/bluebird/js/release/promise.js:729:18)
    at Promise._fulfill (/path/to/node_modules/bluebird/js/release/promise.js:673:18)
    at Promise._resolveCallback (/path/to/node_modules/bluebird/js/release/promise.js:466:57)
    at Promise._settlePromiseFromHandler (/path/to/node_modules/bluebird/js/release/promise.js:559:17)
    at Promise._settlePromise (/path/to/node_modules/bluebird/js/release/promise.js:604:18)
    at Promise._settlePromise0 (/path/to/node_modules/bluebird/js/release/promise.js:649:10)

It works fine when not using twig as docs engine.

@mihkeleidast
Copy link
Member

This can be currently worked around by setting the docs adapter first, then the components adapter. This is also documented in the readme:

However, due to the way this adapter currently extends Twig, it is necessary to set the docs engine before setting the components engine.

However, I'll keep this open since this behavior is certainly not normal and should be fixed at some point.

@danbrellis
Copy link

danbrellis commented May 18, 2022

Also, this change might break something - like when some component handles are used in docs files.

Hey @mihkeleidast , the PR #887 does indeed cause errors when including components in doc files when twig is used as the engine for both components and docs:

Error: Template @my-component not found

I dug into this and it seems that when you are on a doc page, the source for the Twig Adapter is set to docs. This means that the views available to self.views are only docs views.

I don't have a good solution for this, however. Assuming, as you say that it isn't needed to register the loader for docs at all (no need to render a doc handle anywhere), we could change the TwigAdapter to set the source specifically to components:

On line 215 of https://github.com/frctl/fractal/blob/main/packages/twig/src/adapter.js

old

const adapter = new TwigAdapter(Twig, source, app, config);

new

const adapter = new TwigAdapter(Twig, app._components, app, config);

I'm happy to open a PR if you think this is the right approach.

Alternatively, I'd vote to just role back the PR #887 and use the work-around you mentioned. That is a better case than it just not working...

@mihkeleidast
Copy link
Member

@danbrellis please file a new issue with a reproduction repository - i might be able to take a look at some point, but would like an uniform implementation across adapters, so would like to add a test case as well. you can file a pr as well, but keep what i said in the previous sentence in mind, e.g. add a test case and see that it works the same across adapters. that said, see #1167 as well, not sure when i'm able to review.

@danbrellis
Copy link

Thanks for the quick reply @mihkeleidast and I understand about #1167 . I can certainly to a reproduction repository, but am a bit lost when it comes to how to write a test for this. I'll start a new issue with what I am able, if it's out of scope I understand. Thanks!

@mihkeleidast
Copy link
Member

we have adapter integration tests in the examples directory, would be good if all the different adapter examples would then have this example docs page and a test/assertion that they all render the same, or at least similar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants