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

fs.copyTpl throws error with recursive globs (**/*) #19

Closed
timkelty opened this issue Jan 16, 2015 · 5 comments · Fixed by #21
Closed

fs.copyTpl throws error with recursive globs (**/*) #19

timkelty opened this issue Jan 16, 2015 · 5 comments · Fixed by #21

Comments

@timkelty
Copy link

Originally posted here: yeoman/generator#735
@blai suggested I post this here, as it may be a mem-fs-editor bug.

Ran into this here: https://github.com/timkelty/generator-craft-plugin

      this.fs.copyTpl(
        this.templatePath('pluginhandle/**/*'),
        this.destinationPath(pluginDest),
        this
      );

This was working rather recently, but with a freshly re-installed yo, I get this:

events.js:72
        throw er; // Unhandled 'error' event
              ^
TypeError: Cannot call method 'toString' of null
    at copy.process (/Users/timkelty/Repos/generator-craft-plugin/node_modules/yeoman-generator/node_modules/mem-fs-editor/actions/copy-tpl.js:11:34)
    at applyProcessingFunc (/Users/timkelty/Repos/generator-craft-plugin/node_modules/yeoman-generator/node_modules/mem-fs-editor/actions/copy.js:12:16)
    at EditionInterface.exports._copySingle (/Users/timkelty/Repos/generator-craft-plugin/node_modules/yeoman-generator/node_modules/mem-fs-editor/actions/copy.js:52:24)
    at EditionInterface.<anonymous> (/Users/timkelty/Repos/generator-craft-plugin/node_modules/yeoman-generator/node_modules/mem-fs-editor/actions/copy.js:37:10)
    at Array.forEach (native)
    at EditionInterface.exports.copy (/Users/timkelty/Repos/generator-craft-plugin/node_modules/yeoman-generator/node_modules/mem-fs-editor/actions/copy.js:34:9)
    at EditionInterface.module.exports [as copyTpl] (/Users/timkelty/Repos/generator-craft-plugin/node_modules/yeoman-generator/node_modules/mem-fs-editor/actions/copy-tpl.js:9:8)
    at module.exports.yeoman.generators.Base.extend.writing.pluginFiles (/Users/timkelty/Repos/generator-craft-plugin/app/index.js:125:15)
    at /Users/timkelty/Repos/generator-craft-plugin/node_modules/yeoman-generator/lib/base.js:406:16
    at processImmediate [as _immediateCallback] (timers.js:345:15)
@Tokimon
Copy link

Tokimon commented Jan 27, 2015

I had this error as well and after some digging it seems that the { nodir: true } glob option in 'mem-fs-editor/actions/copy.js' (line 33) doesn't work. Because the following var files = glob.sync(from, globOptions) (line 34) consists of both files and folders, which in turn breaks the processing call in the _copySingle method (line 60) as it expects every entry to be a file.

@blai
Copy link
Collaborator

blai commented Jan 27, 2015

@Tokimon Would you be able to come up with a test case that represent your error scenario? I'd be easier to understand your problem in that case.

@markjbyrne81
Copy link

I can confirm @Tokimon issue.

mem-fs-editor\actions\copyTpl.js does not include a globOptions property in the 3rd argument when it calls this.copy (line 9). This means that when _.extend is called in mem-fs-editor\actions\copy.js (line 30) options.globOptions is undefined and in turn _.extend returns undefined instead of { nodir: true }

This then results in directories being returned from the glob.sync() call (line 31) which in turn causes the process function in mem-fs-editor\actions\copyTpl.js (line 11) to fail when it calls contents.toString().

I believe that a change to \actions\copyTpl.js (shown below) could correct this issue. It just adds a globOptions property (set to and empty object) to the object being passed in the 3rd argument of this.copy

'use strict';

var path = require('path');
var _ = require('lodash');

module.exports = function (from, to, context, tplSettings) {
  context = context || {};
  tplSettings = tplSettings || {};
  this.copy(from, to, {
    process: function (contents) {
      return _.template(contents.toString(), context, tplSettings);
    },
    globOptions: {}
  });
};

@SBoudrias
Copy link
Owner

Hey guys, seems you found the issue. So a PR is welcomed to fix it! (with unit test)

@markjbyrne81
Copy link

Hey @SBoudrias . I have never done a PR before but I will happily give it a go later tonight.

blai added a commit to blai/mem-fs-editor that referenced this issue Jan 28, 2015
blai added a commit to blai/mem-fs-editor that referenced this issue Jan 28, 2015
blai added a commit to blai/mem-fs-editor that referenced this issue Jan 28, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants