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

variables in file includes #1990

Closed
acidjazz opened this issue Jun 10, 2015 · 20 comments
Closed

variables in file includes #1990

acidjazz opened this issue Jun 10, 2015 · 20 comments

Comments

@acidjazz
Copy link
Contributor

Is it possible in the future to incorporate something like this

  .sections
    each how, index in self.c.howitworks
      .section(class="section_#{how.name}")
        .icon
          include "../../pub/svg/#{how.name}.svg"
        .copy #{how.copy}
        .cta SEE MORE

(referring to the variable in the include string)

@acidjazz
Copy link
Contributor Author

I'm not able to assign labels, can someone mark this as a feature? or feature request?

@jeromew
Copy link
Contributor

jeromew commented Jun 10, 2015

There is a long standing discussion about dynamic includes. This is not possible in the jade 1.x branch because includes are included at compilation time.

This feature will probably make its way in the 2.x branch in a few months.

don't take this as an offense but I am going to close this issue (please take my word that the level of demand for this feature is already pretty high ;-)).

@jeromew jeromew closed this as completed Jun 10, 2015
@acidjazz
Copy link
Contributor Author

No worries thanks for the quick response, can we then just give this a 2.x label? or is there a list elsewhere like a 2.x wishlist?

@timaschew
Copy link

Is there really a reason to do that?
IMO there is always a solution without dynamic include.

Instead of using a variable in the include you could transform the content of the include into a mixin and if you need some more complex to pass to the mixin, just pass the block, for instance

include mixins

.sections
    each how, index in self.c.howitworks
      .section(class="section_#{how.name}")
        .icon
          +svgIcon(how.name)
        .copy #{how.copy}
        .cta SEE MORE

.section
  h1 or more complex structures
    +form(arg1, arg2)
      input#1
      input#2

@acidjazz
Copy link
Contributor Author

@timaschew could you also give me an example of how the svgIcon mixin would be written?

The main reason is purely dynamic data, for some projects I try to separate copy and sections into modifiable formats, like yaml or markdown. This makes things a breeze when I have copywriters and content managers involved.

@TimothyGu
Copy link
Member

@acidjazz

mixin svgIcon(icon)
  case icon
    when 'a.svg'
      | <svg>blahbl;ah</svg>
    when 'b.svg'
      | <svg>blahblah</svg>
cat <<EOF >>mixins
mixin svgIcon(icon)
  case icon
EOF
for a in `ls svgs`
do
  echo "    when '$a'" >>mixins
  sed 's/^/      | /g' svgs/$a >>mixins
done

@acidjazz
Copy link
Contributor Author

Thanks @TimothyGu while that technically would work for that particular case do you still see how this feature would be awesome?

@TimothyGu
Copy link
Member

@acidjazz No, to be honest I do not. Includes should be static. It would be stupid to do several FS calls for inclusion on every template function run. Now, even if the included templates are cached, it is still going to be a major performance hit since they are not inlined.

The solution provided by @timaschew works, but I would not do it in production simply because, well, you are putting so many unnecessary branches into something that should be static. I was just trying to answer your question on the technical possibility of doing that.

@timaschew
Copy link

I don't know how many svg you have, but you could put all the svgs into one jade file and provide a mixin where you can pass a name, which will be mapped to a specific svg

@jeromew
Copy link
Contributor

jeromew commented Jun 18, 2015

another option is to have

mixin svg_a()
  | <svg>a</svg>
mixin svg_b()
  | <svg>b</svg>

- svg_id = "a";
+#{"svg_" + svg_id}

with this code (dynamic mixin), the rendering is not inlined, but you avoid the switch case and have only one function call, calling the implementation of the mixin

@acidjazz
Copy link
Contributor Author

That still means I maintain a separate list/switch/if scenario either in the mixin or the seperate jade file which is not really ideal. Some projects I work on include 40-100 .svg icons, and I need to include them instead of img(src=) because I can directly manipulate their elements via CSS. Now managing alot of these areas include dynamically generating the content, say from something like,

[
  {
    "name": "Individual",
    "price": 7.99,
    "year": 79,
    "save": 16.88,
    "icon": "single.svg",
    "copy": "Full prevention, detection and resolution services for one adult"
  },
  {
    "name": "Family",
    "price": 9.99,
    "year": 99,
    "save": 20.88,
    "icon": "family.svg",
    "copy": "Full prevention, detection and resolution services for two adults, plus identity monitoring and reoslution services for up to 4 children or other dependants"
  },
  {
    "name": "Couple",
    "price": 8.99,
    "year": 89,
    "save": 18.88,
    "icon": "couple.svg",
    "copy": "Full prevention, detection and resolution services for two adults"
  }
]

Having my content in this format gives me numerous ways to allow other people of the team to tweak prices and copy/etc and swap/modify the icons w/out me being involved.

@TimothyGu I see so you're coming from the perspective of Jades impact on compilation. Is it possible to compile in the order of rendering variables throughout before pulling in includes?

@timaschew
Copy link

do you need the content in a json?
otherwise you coul

icons.jade:

mixin svg(name, price, year, save, copy)
  //- create your markup
  div(title=copy)
    span=name
    div.icon
      //- inject the icon
      block

mixin svg_single
  +svg('Individual', 
    '7.99', 
    '79', 
    '16.88', 
    'Full prevention, detection and resolution services for one adult'
    )
      include 'single.svg'

mixin svg_family
  +svg('Family', 
    '9.99', 
    '99', 
    '20.88', 
    'Full prevention, detection and resolution services for two adults, plus identity monitoring and reoslution services for up to 4 children or other dependants'
    )
      include 'family.svg'

then you can call it like this

include ./icons

+svg_single()
+svg_family()

//- or like pass the local 'svgName' like in jeromew example
+#{"svg_" + svgName}

if you don't want to put the data structure into a jade file, then you need to use a pre-compile step (before jade) and generating the jade templates from your data structure and compiling them in the next build step.

@acidjazz
Copy link
Contributor Author

@timaschew yes, wether it be JSON, or YAML, or whatever, the point is that the content is dynamic. for that particular case it would be the client wanted complete flexibility on adding more pricing packages/etc, and this method allows less to no work in order to support that using jade.

Stepping back here I want to more bring up the general request for this feature of variables in includes, I think it would be awesome to be able to do this, and why we might not be in agreement on our general layouts and structures of what we do, I think the feature would be great to have either way.

So is this realistic for Jade in the future? How intense is the impact? Could it possibly be something that's toggled?

@alubbe
Copy link
Member

alubbe commented Jun 19, 2015

Similar to the other voices, my pondering also yields that we shouldn't have this. I'll outline my reasons as well as yet another alternative for you to use.

My first big beef with it is that it would mean a feature disparity between server-side and client-side jade. Thus, the feature would be used by only part of the community and might have to live behind some config flag.
Moreover, It would entail a departure from being able to run compiled Jade in any JavaScript environment because now we would have a hard dependency on some way to read from the file system. Most likely, this would turn into a hard dependency on node.

Secondly, you have the challenge of sharing variable scope. You want this dynamic include to behave like the other include, so you would expect to access the same variables - which you can because we just inline everything right now (this also yields maximum performance). Going dynamic would mean using mixin-like function calls into which we would have to somehow inject our entire scope, incl. your own local variables that you may have defined in jade code blocks. I'm pretty sure we don't build an JS-AST inside our jade-AST, so that would be the biggest technical challenge.

Lastly, I believe this approach is wrong. I believe jade is a purely view-rendering tool and as such should not concern itself with post-compilation file system access.

I do believe that @TimothyGu and @timaschew already pointed out a great solution of using mixins to solve your problem. This does assume your client is willing to edit and maintain a jade file within the app's repo.

Should that not be the case, my suggestion would be to enrich the array you provided before you call the jade view:

var svgs = {};
function loadSvg(fileName) {
  if(svgs[fileName]) return svgs[fileName];
  else return svgs[fileName] = fs.fileReadSync(fileName, "utf8");
}
function enrichData(data) {
  for(var i = 0, var l = data.length; i < l; i++) data[i].svgData = loadSvg(data[i].icon);
}

@acidjazz
Copy link
Contributor Author

@alubbe I understand the complications this might present in Jade for this, (potential slower compilations, a feature not usable for front end compilation, this possibly being toggle-able etc).

I don't agree that I've been given realistic solutions for my case, the 1st solution derails my point of having the content completely controllable by the data brought into jade, and the second doesn't seem comforting for production, dynamically generating a mixin via shell. (although still impressive).

I see your point on how jade is purely view-rendering and the deny of post-compilation file access. But I still think this feature would be awesome and very helpful. From the users perspective it's quite simple, you can use variables in html, you can use variables in element properties, you can loop through objects and arrays, you have full access to manipulate them w/ lease w/ JS, but stay away from includes?

Although we might generally disagree on how we use jade, I still think it would great to be able to go that route, and I think some others would like it as well

http://stackoverflow.com/questions/12132978/use-a-variable-in-a-jade-include
http://stackoverflow.com/questions/16261779/jade-include-template-based-on-a-variable
http://stackoverflow.com/questions/16579461/jade-doesnt-let-me-use-include-variable-in-a-mixin

It seemed TJ was fine with adding it :
#282

#839
#1897
#1924

Some others (might have some duplicates):

#416
#569
#839
#843
#1312
#1433
#1481
#1893
#1924

@Nikolasgrizli
Copy link

I found another way - svg sprites!
I have this structure:
-pug
--index.pug
-images
--sprites
---svg
----.keep
----electrics.svg
----kitchen.svg

index.pug
at start - "coverage":[{ "name":"svg icon", "pathSvg": "electrics"}, { "name":"another svg", "pathSvg": "kitchen"}, ]
at layput -
each val in coverage p #{val.name} +svg(${val.pathSvg})

+svg = this mixin
mixin svg(name) svg&attributes(attributes) use(xlink:href=${baseDir}images/sprites.svg#${name})

@mattpr
Copy link

mattpr commented Jan 24, 2019

I get that includes are done at compile time and so variable interpolation in includes might cause confusion for nodejs devs. In my current use case I am using pug for templating a static site (built with gulp). So the compilation only happens once per deployment. I am building with something like

gulp build --prod --lang en

and then compiling the pug templates with gulp-pug with something like:

...
.pipe(pug({
    data: {
        lang: argv.lang || 'en',
    }
}))
...

Would be convenient to be able to do:

include content/#{lang}/mission.pug

Instead of

if lang == 'en'
    include content/en/mission.pug
else if lang == 'de'
    include content/de/mission.pug
else...

...for every included section (there are a fair number of content snippets being included in each language build).

I am sure there is a more elegant way, or I just have to use some gulp search/replace plugin after the pug compilation.

I guess the core issue here is that normal pug variables are used within the compiled template (ie after includes are done) as the template is used? In my case I only compile once and it doesn't make sense to ever use pug variables after compile time since I am rendering static html files. The fact I can do includes within a conditional based on a variable confuses things a bit though I think.

I know you guys decided against supporting this, but figured I would share my use-case.

@edoardoo
Copy link

edoardoo commented Jan 4, 2020

I use pug for templating a static site too and not having the possibility to include files (pug, svg...) using a variable is incredibly frustrating.
The lack of these features it's forcing me to use shenanigans which will probably lead to unmaintainable code.
Finally, finding so many old issues and SO posts begging for this feature is making me reconsider the adoption of the framework itself unfortunately, even though I liked it very much.

@acidjazz
Copy link
Contributor Author

acidjazz commented Jan 6, 2020

@edoardoo it has been a minute since we've addressed this issue, but these days i've been using pug w/ vuejs (nuxtjs) which allows me to separate my templates at the layout/component level.

Don't know if this might help you reconsider

ghost pushed a commit to EU-EDPS/website-evidence-collector that referenced this issue Mar 19, 2020
the previous method breaks when installing the package with yarn, 
because yarn installs dependencies into a different folder structure.

As pugjs does not support dynamic includes, file content is loaded as 
unescaped plain text into template.

ref: pugjs/pug#1990
ghost pushed a commit to EU-EDPS/website-evidence-collector that referenced this issue Mar 19, 2020
the previous method breaks when installing the package with yarn, 
because yarn installs dependencies into a different folder structure.

As pugjs does not support dynamic includes, file content is loaded as 
unescaped plain text into template.

ref: pugjs/pug#1990
ghost pushed a commit to EU-EDPS/website-evidence-collector that referenced this issue Mar 25, 2020
the previous method breaks when installing the package with yarn, 
because yarn installs dependencies into a different folder structure.

As pugjs does not support dynamic includes, file content is loaded as 
unescaped plain text into template.

ref: pugjs/pug#1990
@dmz9
Copy link

dmz9 commented May 21, 2020

a small workaround for those who still looking for a solution to include all your mixins in one place without future refactoring.
since we cant use dynamic includes - lets generate a file with all recursive includes before gulp pipeline starts.
this snippet will also work if gulpfile split by multiple files and build function is located outside of index.js.
i used extra package fs-readdir-recursive for reading files recursively, but code of the module is not big so you can write it yourself if needed

// gulpfile.js/tasks/pug-build.js
const fs = require('fs'),
    gulp = require('gulp'),
    ...

export default function() {
    // path to mixins directory, relative to gulpfile.js
    const mixinsPath = process.cwd() + '/src/pug/mixins';
    // desired file name for generated pug file with all includes
    const includeFileName = 'all-mixins-generated.pug'
    // read all filenames recursively inside mixinsPath
    // including only *.pug files, and excluding the generated file itself
    const mixins = require('fs-readdir-recursive')(mixinsPath)
               .filter(filename => /\.pug$/.test(filename))
               .filter(filename => includeFileName !== filename)
               .map(filename => `include ${filename}`)
               .join("\n")
    fs.writeFileSync(`${mixinsPath}/${includeFileName}`, mixins)

    return gulp
               .src('src/**/*.pug')
               .pipe( ... )
               ...
// src/pug/templates/base-template.pug 

include ../mixins/all-mixins-generated.pug

doctype html
// ...

@pugjs pugjs locked and limited conversation to collaborators May 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants