twigjs / twig.js

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

{{ dump() }} doesn't work #533

Closed mrleblanc101 closed 6 years ago

mrleblanc101 commented 6 years ago

Hi, I'm trying to use {{ dump() }} all variables from the context but it doesn't work. Kills my gulp and doesn't render the template. If i use {{ dump(var) }} it works but i'd have to test for all variables.

<--- Last few GCs --->

[86493:0x103000000] 62599 ms: Mark-sweep 1416.1 (1469.3) -> 1415.9 (1453.3) MB, 7077.9 / 0.0 ms (+ 0.0 ms in 0 steps since start of marking, biggest step 0.0 ms, walltime since start of marking 7078 ms) last resort [86493:0x103000000] 73959 ms: Mark-sweep 1415.9 (1453.3) -> 1415.9 (1453.3) MB, 11359.6 / 0.0 ms last resort

<--- JS stacktrace --->

==== JS stack trace =========================================

Security context: 0x1e412631ba79 2: dumpVar [/Users/sleblanc/www/sagrada/node_modules/twig/twig.js:~4075] [pc=0x35f88057778c](this=0x3e4f3fa8a1e1 ,variable=0x2c6a47e90b9 <a Stats with map 0x373335157449>) 3: displayVar [/Users/sleblanc/www/sagrada/node_modules/twig/twig.js:~4061] [pc=0x35f880576c4d](this=0x3e4f3fa8a1e1 ,variable=0x2c6a47e90b9 <a Stats with map 0x373335157449>) 4:...

FATAL ERROR: CALL_AND_RETRY_LAST Allocation failed - JavaScript heap out of memory 1: node::Abort() [/usr/local/bin/node] 2: node::FatalException(v8::Isolate, v8::Local, v8::Local) [/usr/local/bin/node] 3: v8::internal::V8::FatalProcessOutOfMemory(char const, bool) [/usr/local/bin/node] 4: v8::internal::Factory::NewFillerObject(int, bool, v8::internal::AllocationSpace) [/usr/local/bin/node] 5: v8::internal::Runtime_AllocateInTargetSpace(int, v8::internal::Object*, v8::internal::Isolate) [/usr/local/bin/node] 6: 0x35f87ec840bd

willrowe commented 6 years ago

Can you post the template that dump call is being made in?

ericmorand commented 6 years ago

And the belonging data. I suspect this is coming from a huge data object or a circular reference in the data.

mrleblanc101 commented 6 years ago

The gulp task:

const build = function() {
    return gulp.src('src/templates/views/*.twig')
        .pipe(data(function(file) {
            try {
                return JSON.parse(fs.readFileSync('src/data/' + path.basename(file.path, '.twig') + '.json'));;
            } catch (err) {
                if (err.code === 'ENOENT') {
                    console.log('No JSON data for view: ' + file);
                    return {};
                } else {
                    throw err;
                }
            }
        }))
        .pipe(data(function(file) {
            var data = fm(String(file.contents));
            file.contents = new Buffer(data.body);
            return data.attributes;
        }))
        .pipe(data(function(file) {
            console.log(file.data)
        }))
        .pipe(twig({
            useFileContents: true,
            functions: [{
                name: "svg",
                func: function(name, width, heigth = width, alt = null) {
                    return `
                    <svg class="icon-${name}" width="${width}" heigth="${heigth}">
                        ${alt ? '<title>${alt}</title>' : ''}
                        <use xlink:href="${name}" />
                    </svg>`;
                }
            }]
        }))
        .pipe(gulp.dest('build/'))
}

The template content:

---
test: "my var"
---
<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <meta http-equiv="X-UA-Compatible" content="ie=edge">
    <title>Document</title>
    {% include 'partials/head.twig' %}
</head>
<body>
    {% include 'partials/header.twig' %}
        {{ dump() }}
    {% include 'partials/footer.twig' %}
</body>
</html>

My json file:

{
    "title": "Gulp",
    "description": "Gulp project starter"
}

The console log result in my console:

{ description: 'Gulp project starter',
  title: 'Gulp',
  test: 'my var' }
mrleblanc101 commented 6 years ago

The problem is in gulp-twig package. Sorry guys !

The plugin create a circular reference by adding the file buffer to the template data object causing a crash when we call {{ dump() }} without arguments.

It's weird because it's seems like the only reason they add _file and _target to the data object is for their unit test which print the relative path of those file in a twig template and compare it to a pre-generated .html file.

They shouldn't append internal data to the user data object, that seems very bad or at least they should only add the relative path to the data object to prevent a circular reference which cause a crash. I created a pull request but the package seems inactive. Also it obviously break their unit test because they were flawed.

Here's my data:

{  
   "Webpack":"4",
   "Gulp":"4",
   "description":"Gulp project",
   "title":"Gulp",
   "test":"test variable"
}

Here's the data twig receive after gulp-twig:

{
    "Webpack": "4",
    "Gulp": "4",
    "description": "Gulp project",
    "title": "Gulp",
    "test": "test variable",
    "_file": < File "index.twig" < Buffer 0 a 7 b 25 20 65 78 74 65 6e 64 73 20 27 2e 2e 2 f 62 61 73 65 2e 74 77 69 67 27 20 25 7 d 0 a 0 a 7 b 25 20 62 6 c 6 f 63 6 b 20 63 6 f 6e 74 65 6e 74 20 25 7 d... >> ,
    "_target": {
        path: '/Users/mrleblanc/GitHub/project-starter/src/templates/views/index.html',
        relative: 'index.html'
    }
}

EDIT: Just tried JSON.stringify the return object in the gulp-twig package and got:

TypeError: Converting circular structure to JSON
    at JSON.stringify (<anonymous>)
    at modifyContents (/Users/mrleblanc/GitHub/project-starter/node_modules/gulp-twig/index.js:40:20)
    at wrappedMapper (/Users/mrleblanc/GitHub/project-starter/node_modules/map-stream/index.js:84:19)
    at Stream.stream.write (/Users/mrleblanc/GitHub/project-starter/node_modules/map-stream/index.js:96:21)
    at DestroyableTransform.ondata (/Users/mrleblanc/GitHub/project-starter/node_modules/through2/node_modules/readable-stream/lib/_stream_readable.js:619:20)
    at emitOne (events.js:115:13)
    at DestroyableTransform.emit (events.js:210:7)
    at addChunk (/Users/mrleblanc/GitHub/project-starter/node_modules/through2/node_modules/readable-stream/lib/_stream_readable.js:291:12)
    at readableAddChunk (/Users/mrleblanc/GitHub/project-starter/node_modules/through2/node_modules/readable-stream/lib/_stream_readable.js:278:11)
    at DestroyableTransform.Readable.push (/Users/mrleblanc/GitHub/project-starter/node_modules/through2/node_modules/readable-stream/lib/_stream_readable.js:245:10)

Thanks for you time @ericmorand @willrowe !

ericmorand commented 6 years ago

Good catch. And what a super bad coding practice from them. Let's hope someone wakeup and merge your PR.

ryuran commented 6 years ago

It's weird because it's seems like the only reason they add _file and _target to the data object is for their unit test which print the relative path of those file in a twig template and compare it to a pre-generated .html file.

I use that to transform relative link in source to relative link into the build directory. Because some, _target and _file are usefull to compare paths. I have a {% root %} helper always returning relative path to build root directory. https://github.com/cleverage/garden-starter-kit/blob/next/tools/twig/extends/root.js

mrleblanc101 commented 6 years ago

@ryuran That's still bad coding practice and invalid data. Can't you just send the relative path as data instead of the actual file creating a circular reference in your JSON ?

dave-irvine commented 6 years ago

@mrleblanc101 do you want to close the issue here in favour of https://github.com/zimmen/gulp-twig/pull/51 ?

mrleblanc101 commented 6 years ago

If you want. But I think we should keep it here as I don’t think my merge request will ever get merged.

dave-irvine commented 6 years ago

@mrleblanc101 is the issue actually in this library, or is it a problem in gulp-twig?

toptalo commented 6 years ago

Just try it with my grunt-twig2html plugin. {{ dump() }} works well. So I think it's gulp-twig issue

mrleblanc101 commented 6 years ago

I use gulp :/

dave-irvine commented 6 years ago

Unfortunately this isn't something we can fix here, its an issue on another project https://github.com/zimmen/gulp-twig/pull/51 so I'm closing as otherwise the issue would be open forever.