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

refactor: enable fileendings separated by more than one dot #1459

Draft
wants to merge 16 commits into
base: dev
Choose a base branch
from

Conversation

mfranzke
Copy link
Contributor

@mfranzke mfranzke commented Dec 27, 2022

Closes #1030

Summary of changes:

  • enhancing fileextensions list in packages/engine-twig/lib/engine_twig.js and packages/engine-twig-php/lib/engine_twig_php.js to even also include .html.twig besides .twig
  • rework in packages/core/src/lib/object_factory.js to check for a configured multi-dotted file extension and remove this form one from variable this.fileName whereas replace it in this.fileExtension

@mfranzke mfranzke self-assigned this Dec 27, 2022
@mfranzke mfranzke linked an issue Dec 27, 2022 that may be closed by this pull request
@mfranzke mfranzke changed the title refactor: enabled fileendings separated with more than one dot refactor: enable fileendings separated with more than one dot Dec 28, 2022
@mfranzke mfranzke marked this pull request as ready for review December 28, 2022 11:13
@@ -129,8 +129,8 @@ const engine_handlebars = {
* assume it's already present
*/
spawnMeta: function (config) {
this.spawnFile(config, '_head.hbs');
this.spawnFile(config, '_foot.hbs');
this.spawnFile(config, '_head.' + config.patternExtension);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be dangerous.
When pattern lab discovers multiple pattern engines, it will try to spawn the file found in the engine's folder located under _meta/head...
In that case, the file will not be found, and an error will be thrown here

const metaFileContent = fs.readFileSync(

You can either catch the error and ignore it, or you revert it to its former state.

Another issue could be, that by chance the file will be taken, but the output for the twig file will be overwritten by the content of the hbs file. In that case, it could lead to an error when serving pattern lab because it has a different rendering engine.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather look into the engineFileExtensions and when the config.patternExtension is included there, output the required file. (In that case, we need to read the file we deliver with the engine from the _meta folder and spawn it as the configured extension in config.patternExtension.

@mfranzke mfranzke marked this pull request as draft December 29, 2022 13:22
@mfranzke mfranzke changed the title refactor: enable fileendings separated with more than one dot refactor: enable fileendings separated by more than one dot Dec 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fully support .html.twig suffix
2 participants