search

pure180 / gulp-pug-inheritance

Gulp plugin to rebuild jade files and all files that have extended or included those files
11 stars 7 forks source link

Persisted inheritance tree issue with fragments #4

Closed daankets closed 3 years ago

daankets commented 7 years ago

I have a fragment called /fragments/scripts.pug, which is used by /index.pug. Within the inheritance temp file, the dependency is recorded as such:

"index_pug": {
    "tree": {
      "index.pug": {}
    },
    "files": [
      "index.pug"
    ],
    "dependencies": [
      "fragments/footer.pug",
      "fragments/scripts.pug"
    ],
    "file": "index.pug"
  }

When changing the scripts.pug file (using a touch), this is supposed to (I believe) ensure that index.pug is added back into the stream after being filtered out (because no changes in the index.pug file). This however does not happen, and after the pug-inheritance step, I'm left with only my fragments to process (which are then filtered out).

pure180 commented 7 years ago

Can you please provide your gulp pug/jade task and are you using the beta version for this task?

daankets commented 7 years ago

I’m using the beta, and the inheritance file is created…

Here is the gulp task:

return gulp
   .src(DASHBOARD_SRC("pug/**/*.pug"))
   .pipe(gulpDebug({title: "before changed filter"}))
   .pipe(gulpChanged(DASHBOARD_BUILD("html"), {extension: ".html"}))
   .pipe(gulpDebug({title: "before cache filter"}))
   .pipe(gulpIf(global.isDashboardPugWatching, gulpCached("pug")))
   .pipe(gulpDebug({title: "before inheritance filter"}))
   .pipe(pugInheritance({
      basedir: DASHBOARD_SRC("pug"),
      skip: [],
      saveInTempFile: true,
      tempFile: DASHBOARD_BUILD(".pugInheritance.json")
   }))
   .pipe(gulpDebug({title: "before fragment filter"}))
   .pipe(gulpFilter((file) => {
      return file.path.indexOf("fragments") === -1;
   }))
   .pipe(gulpDebug({title: "before pug"}))
   .pipe(pug({
      pretty: true
   }))
   .pipe(gulp.dest(DASHBOARD_BUILD("html")));
daankets commented 7 years ago

I think the problem starts with filtering all the fragments by default. It's better to also build them so you can compare the changed date/time. Then, the second filter part can be dropped.

daankets commented 7 years ago

Changed the flow as described above: now also building the fragments, and storing the results (no longer filtering fragments after). However, the dependencies of the fragments are NOT re-inserted into the stream.

pure180 commented 7 years ago

Sure, i will have a closer look on it. Maybe there is something wrong with the returned eventstream, it should pass all concerning files back to the stream.

What is the result of: gulpDebug({title: "before inheritance filter"}) and gulpDebug({title: "before fragment filter"})

daankets commented 7 years ago

Before the inheritance filter: 1 file (scripts.pug) After: 1 file (scripts.pug) Expected: 2 files (scripts.pug, index.pug)

I'm debugging now...

daankets commented 7 years ago

Ok. I think I've got it... I think the concept of the flow is a bit wrong...

If I have it right, the current way this works is by checking all dependencies for a file, and pulling them into the build. For example, if file a is changed, and a depends on b &c, b and c will be added to the build. That's not really required however, as pug will re-build them anyway (they are included). Instead, we should check which items are impacted by a rebuild of a, and add those to the downstream build. A problem is that each item can again cause other items to be added, so we may need to exclude duplicates.

pure180 commented 7 years ago

Yes pug-inheritance takes all existing files and resolves the tree by using pug-walk, it checks the AST types of Exclude and Include. If gulp-pug-inheritance is used as mentioned in the readme and it is in watch-state, it only gets the previously cached and changed files. But pug-inheritance is resolving all files that are included inside the set basedir. gulp-pug-inheritance resolves the dependencies and inheritances of the passed file, creates an array of all affected files (Line 192) and returns them as a new file buffer as stream back to the gulp-task (Line 196). This stream will be filtered and passed to gulp-pug then afterwards..

pure180 commented 7 years ago

Okey, so lets break this down. Assumed we have this kind of file structure:

./app/_extends/layout.pug
  -- includes section1.pug
./app/_includes/section1.pug
./app/_includes/section2.pug
./app/_includes/section3.pug
./app/index.pug
  -- extends layout.pug
  -- includes section2.pug and section3.pug

Using this gulp-tasks:

gulp.task('jade', function() {
    return gulp.src('app/**/*.pug')
        .pipe(
          plumber( function(error) {
            gutil.log(error.message);
            this.emit('end');
          })
        )
        .pipe(changed('dist', {extension: '.html'}))
        .pipe(gulpif(global.isWatching, cached('jade')))
        .pipe(debug({title: 'Debug before gulp-pug-inheritance'}))
        .pipe(pugInheritance({basedir: 'app', extension: '.pug', skip:'node_modules', saveInTempFile: true}))
        .pipe(debug({title: 'Debug after gulp-pug-inheritance'}))
        .pipe(filter(function (file) {
            return !/\/_/.test(file.path) && !/^_/.test(file.relative);
        }))
        .pipe(debug({title: 'Debug after gulp-filter'}))
        .pipe(jade())
        .pipe(gulp.dest('dist'))
        .pipe(notify({message: 'Served "<%= file.path %>"'}));
});
gulp.task('setWatch', function() {
    global.isWatching = true;
});
gulp.task('watch', ['setWatch', 'jade'], function() {
    gulp.watch( 'app/**/*.pug', ['jade']);
});

The result of the first run will be:

[19:32:29] Using gulpfile H:\GitHub\gulp-pug-inheritance-test\gulpfile.js
[19:32:29] Starting 'setWatch'...
[19:32:29] Finished 'setWatch' after 93 μs
[19:32:29] Starting 'jade'...
[19:32:29] Debug before gulp-pug-inheritance app\index.pug
[19:32:29] Debug before gulp-pug-inheritance app\_extends\layout.pug
[19:32:29] Debug before gulp-pug-inheritance app\_includes\section1.pug
[19:32:29] Debug before gulp-pug-inheritance app\_includes\section2.pug
[19:32:29] Debug before gulp-pug-inheritance app\_includes\section3.pug
[19:32:29] Debug before gulp-pug-inheritance 5 items
[gulp-pug-inheritance] Plugin started for the first time. Save inheritances to a tempfile
[gulp-pug-inheritance][NEW] Get inheritance of: "index.pug" - 22ms
[gulp-pug-inheritance][NEW] Get inheritance of: "_extends\layout.pug" - 28ms
[gulp-pug-inheritance][NEW] Get inheritance of: "_includes\section1.pug" - 16ms
[gulp-pug-inheritance][NEW] Get inheritance of: "_includes\section2.pug" - 16ms
[gulp-pug-inheritance][NEW] Get inheritance of: "_includes\section3.pug" - 0ms
[19:32:30] Debug after gulp-pug-inheritance app\index.pug
[19:32:30] Debug after gulp-filter app\index.pug
[19:32:30] Debug after gulp-pug-inheritance app\_extends\layout.pug
[19:32:30] Debug after gulp-pug-inheritance app\_includes\section1.pug
[19:32:30] Debug after gulp-pug-inheritance app\_includes\section2.pug
[19:32:30] Debug after gulp-pug-inheritance app\_includes\section3.pug
[19:32:30] Debug after gulp-pug-inheritance 5 items
[19:32:30] Debug after gulp-filter 1 item
[19:32:30] gulp-notify: [Gulp notification] Served "H:\GitHub\gulp-pug-inheritance-test\dist\index.html"
[19:32:30] Finished 'jade' after 233 ms
[19:32:30] Starting 'watch'...
[19:32:30] Finished 'watch' after 15 ms

Then making changes to ./app/_includes/section1.pug will log:

[19:35:39] Starting 'jade'...
[19:35:39] Debug before gulp-pug-inheritance app\_includes\section1.pug
[19:35:39] Debug before gulp-pug-inheritance 1 item
[gulp-pug-inheritance] Plugin already started once. Get inheritances from a tempfile
[gulp-pug-inheritance][CACHED] Get inheritance of: "_includes\section1.pug" - 6ms
[19:35:39] Debug after gulp-pug-inheritance app\_includes\section1.pug
[19:35:39] Debug after gulp-pug-inheritance app\_extends\layout.pug
[19:35:39] Debug after gulp-pug-inheritance app\index.pug
[19:35:39] Debug after gulp-filter app\index.pug
[19:35:39] Debug after gulp-pug-inheritance 3 items
[19:35:39] Debug after gulp-filter 1 item
[19:35:39] gulp-notify: [Gulp notification] Served "H:\GitHub\gulp-pug-inheritance-test\dist\index.html"
[19:35:39] Finished 'jade' after 80 ms

Then including ./app/_includes/section1.pug to ./app/index.pug logs:

[19:40:24] Starting 'jade'...
[19:40:24] Debug before gulp-pug-inheritance app\index.pug
[19:40:24] Debug before gulp-pug-inheritance 1 item
[gulp-pug-inheritance] Plugin already started once. Get inheritances from a tempfile
[gulp-pug-inheritance][CACHED] Get inheritance of: "index.pug" - 2ms
[19:40:24] Debug after gulp-pug-inheritance app\index.pug
[19:40:24] Debug after gulp-filter app\index.pug
[19:40:24] Debug after gulp-pug-inheritance 1 item
[19:40:24] Debug after gulp-filter 1 item
[19:40:24] gulp-notify: [Gulp notification] Served "H:\GitHub\gulp-pug-inheritance-test\dist\index.html"
[19:40:24] Finished 'jade' after 80 ms

The result of temp.pugInheritance.json after the changes: 

{
  "index_pug": {
    "tree": {
      "index.pug": {}
    },
    "files": [
      "index.pug"
    ],
    "dependencies": [
      "_extends\\layout.pug",
      "_includes\\section2.pug",
      "_includes\\section3.pug",
      "_includes\\section1.pug"
    ],
    "file": "index.pug"
  },
  "_extends_layout_pug": {
    "tree": {
      "_extends\\layout.pug": {
        "extendedBy": [
          {
            "index.pug": {}
          }
        ]
      }
    },
    "files": [
      "_extends\\layout.pug",
      "index.pug"
    ],
    "dependencies": [
      "_includes\\section1.pug"
    ],
    "file": "_extends\\layout.pug"
  },
  "_includes_section1_pug": {
    "tree": {
      "_includes\\section1.pug": {
        "includedBy": [
          {
            "_extends\\layout.pug": {
              "extendedBy": [
                {
                  "index.pug": {}
                }
              ]
            }
          }
        ]
      }
    },
    "files": [
      "_includes\\section1.pug",
      "_extends\\layout.pug",
      "index.pug"
    ],
    "dependencies": [],
    "file": "_includes\\section1.pug"
  },
  "_includes_section2_pug": {
    "tree": {
      "_includes\\section2.pug": {
        "includedBy": [
          {
            "index.pug": {}
          }
        ]
      }
    },
    "files": [
      "_includes\\section2.pug",
      "index.pug"
    ],
    "dependencies": [],
    "file": "_includes\\section2.pug"
  },
  "_includes_section3_pug": {
    "tree": {
      "_includes\\section3.pug": {
        "includedBy": [
          {
            "index.pug": {}
          }
        ]
      }
    },
    "files": [
      "_includes\\section3.pug",
      "index.pug"
    ],
    "dependencies": [],
    "file": "_includes\\section3.pug"
  }
}

For me this looks like exepted, or am i wrong?

daankets commented 7 years ago

Yes, this seems right. Strange, I don't see the same behaviour... For me, an included file is modified, enters the stream, and goes through the inheritance pipe but no extra files come out... (although the inheritance file clearly lists it as an inheritance) I'll debug once more....

pure180 commented 7 years ago

I can add a log to the last instance of the plugin, at the position, when the files are passed back to the stream if needed? Btw for testing purpose you can also do this yourself in ./node_modules/gulp-pug-inheritance folder.

daankets commented 7 years ago

Just did that ;-)

On 13 Mar 2017, at 21:25, Daniel Pfisterer notifications@github.com wrote:

I can add a log to the last instance of the plugin, at the position, when the files are passed back to the stream if needed? Btw for testing purpose you can also do this yourself in ./node_modules/gulp-pug-inheritance folder.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/pure180/gulp-pug-inheritance/issues/4#issuecomment-286216207, or mute the thread https://github.com/notifications/unsubscribe-auth/ACLKMRDYuz5Srj8Q36LCmPMOYeYcrbGsks5rlZhBgaJpZM4MalnD.

-- Disclaimer This message is confidential and only intended for the originally intended recipient(s). It may also be privileged or otherwise protected by work product immunity or other legal rules. If you think you have received this message by mistake, please let us know by e-mail reply and delete it from your system; you must not copy this message or disclose its contents to anyone other than the intended recipients.

pure180 commented 7 years ago

Can you share the affected repo, so I may take a look on it? :-)

daankets commented 7 years ago

No, I’m afraid not ;-) It’s a corporate one… But thanks for the offer ;-)

On 13 Mar 2017, at 21:33, Daniel Pfisterer notifications@github.com wrote:

Can you share the affected repo, so I may take a look on it? :-)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/pure180/gulp-pug-inheritance/issues/4#issuecomment-286218280, or mute the thread https://github.com/notifications/unsubscribe-auth/ACLKMQ7bcL0lnVuYCqjt5P2MH1QwhW8Bks5rlZoNgaJpZM4MalnD.

-- Disclaimer This message is confidential and only intended for the originally intended recipient(s). It may also be privileged or otherwise protected by work product immunity or other legal rules. If you think you have received this message by mistake, please let us know by e-mail reply and delete it from your system; you must not copy this message or disclose its contents to anyone other than the intended recipients.

pure180 commented 7 years ago

Well that's a problem of the none open source applications. ;-)

daankets commented 7 years ago

Sure, I’ll tell my customer ;-)

On 13 Mar 2017, at 21:40, Daniel Pfisterer notifications@github.com wrote:

Well that's a problem of the none open source applications. ;-)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/pure180/gulp-pug-inheritance/issues/4#issuecomment-286220069, or mute the thread https://github.com/notifications/unsubscribe-auth/ACLKMZ-9QrYteLGmmPneyR_ppPowD-_nks5rlZuRgaJpZM4MalnD.

-- Disclaimer This message is confidential and only intended for the originally intended recipient(s). It may also be privileged or otherwise protected by work product immunity or other legal rules. If you think you have received this message by mistake, please let us know by e-mail reply and delete it from your system; you must not copy this message or disclose its contents to anyone other than the intended recipients.

daankets commented 7 years ago

I was wondering about something by the way... Wouldn't it be an idea to contribute an addition to the gulp-pug and pug projects, that would take over the work of building the dependency tree during the pug build? Pug has to walk the dependencies during the build anyway. If it could take an option {buildDepTree:"destinationFilePath"}, it could do so while building - therefore removing the need for an additional walker. The build would not slow down noticeably I think, and the tree can be used for dependency detection in every subsequent pass.

pure180 commented 7 years ago

First of all I guess this is against the philosophy of the gulp plugin guidelines. They highly recommend to follow them. As they mean that each plugin should do just one task, gulp-pug is there to compile pug files. I also think if the creators of jade needed to create inheritances of the pug files, they also would have coded it already. For me the way gulp-pug-inharitance is working is just fine and I'm using it all day in work too.

pure180 commented 7 years ago

@daankets Has you been able to find the bug or resolving your problem?

daankets commented 7 years ago

Hi Daniel

No, not yet (I’m actually traveling now, and in our offices in Bulgaria… I’ll have time again later this week).

With kind regards

Daan Kets Managing Partner Delta Source Belgium

On 15 March 2017 at 10:37:09, Daniel Pfisterer (notifications@github.com) wrote:

@daankets https://github.com/daankets Has you been able to find the bug or resolving your problem?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pure180/gulp-pug-inheritance/issues/4#issuecomment-286674718, or mute the thread https://github.com/notifications/unsubscribe-auth/ACLKMZGN0i5-T_i9SB4j2W_uqIw3OwB0ks5rl6M1gaJpZM4MalnD .

-- Disclaimer This message is confidential and only intended for the originally intended recipient(s). It may also be privileged or otherwise protected by work product immunity or other legal rules. If you think you have received this message by mistake, please let us know by e-mail reply and delete it from your system; you must not copy this message or disclose its contents to anyone other than the intended recipients.

pure180 commented 7 years ago

Okey, so waiting for your response. Have a safe journey!

pure180 commented 7 years ago

Guess I found the bug, inheritances of new dependencies hasn't been updated after change. When recaching the inheritance-tree of the affected file, e.g.: index.pug includes or extendes a new file, the tree of the new file wasn't updated, so it is ignored on end stream, because there didn't exist any known inheritances at this point.

Changed this in the current release candidate. Can you please check it, if you have time? (Please remove the tempfile before running your task)

gulp-pug-inheritance@1.0.0-rc.3

daankets commented 7 years ago

Will do so ;-)

On 17 Mar 2017, at 10:21, Daniel Pfisterer notifications@github.com wrote:

1.0.0-rc.3

-- Disclaimer This message is confidential and only intended for the originally intended recipient(s). It may also be privileged or otherwise protected by work product immunity or other legal rules. If you think you have received this message by mistake, please let us know by e-mail reply and delete it from your system; you must not copy this message or disclose its contents to anyone other than the intended recipients.

pure180 commented 7 years ago

Damn forgot to update dependencies in package.json. Its now gulp-pug-inheritance@1.0.0-rc.4

daankets commented 7 years ago

Seems to work fine now, I will test further this evening ;-)

On 17 Mar 2017, at 10:21, Daniel Pfisterer notifications@github.com wrote:

gulp-pug-inheritance

-- Disclaimer This message is confidential and only intended for the originally intended recipient(s). It may also be privileged or otherwise protected by work product immunity or other legal rules. If you think you have received this message by mistake, please let us know by e-mail reply and delete it from your system; you must not copy this message or disclose its contents to anyone other than the intended recipients.

pure180 commented 7 years ago

This sounds great! :-) please let me know if there are still issues. I'm planing to publish this version on next weekend.