gregnb / filemanager-webpack-plugin

Copy, move, archive (zip/tar/tar.gz), delete files and directories before and after Webpack builds. Win32/Mac/*Nix supported
MIT License
462 stars 34 forks source link

Cannot read property 'isFile' of undefined #12

Closed henryruhs closed 6 years ago

henryruhs commented 6 years ago

This plugin is not working since the update... you are not going to become a lucky guy without adding unit testing to this project :-)

Config

copy: [
   {
    source: './node_modules/svg-country-flags/svg/{de,gb-nir,fr}.svg',
    destination: './dist/assets/'
   }
]]

Error

TypeError: Cannot read property 'isFile' of undefined
    at /home/redaxmedia/Projekte/***/node_modules/filemanager-webpack-plugin/src/index.js:124:28
gregnb commented 6 years ago

Is that filename correct? What OS is this? Yes, I know. Unit tests are on the todo list ;)

henryruhs commented 6 years ago

The files are valid - well it is an pattern. Should copy de.svg, gb-nir.svg and fr.svg to the dist.

gregnb commented 6 years ago

Ok, I see the issue now. Thanks for highlighting the problem. It was confusing at first because no way you can issue a standard unix copy pattern like that and it work. Something cpx does.

I will look into it. I'll also probably turn what you posted above into a template for ISSUES. I'll also roll in the tests over the weekend since it seems like more people are using this package now.

henryruhs commented 6 years ago

Great, thanks for your putting work into this plugin. Have a great weekend.

gregnb commented 6 years ago

One more thing, can you tell me which version last worked for you so I can take a look at that and compare against the new to be sure that this was working?

henryruhs commented 6 years ago

Last version I used is 1.0.11 but I think that does not help as there is no cpx inside :-)

gregnb commented 6 years ago

Yeah that's why I'm confused. The package used cpr before and I didn't think it had the ability to read a source file/dir in the format you have above:

'./node_modules/svg-country-flags/svg/de,gb-nir,fr}.svg',

Looking at cpr I can see it handles something similar but not exactly the same: $ cpx "src/*/.{html,png,jpg}" app -w & watchify src/index.js -o app/index.js

But I'll give it a shot when I work on this later

henryruhs commented 6 years ago

There was a typo... {de,gb-nir,fr}.svg is correct. It should work, but there is a check isFile() that fails on a wildcard path I think.

gregnb commented 6 years ago

Alright, I made a fix for this issue and I added in unit tests (finally!). I updated the README to also reflect the fact that you can pass in {} for the source file thanks to cpx.

Unit tests have been long overdue. Sorry for any disruptions on your end! Let me know if this latest version works for you

weotch commented 6 years ago

I'm still having the issue on 1.0.17 on MacOs, Node 8.9.0

gregnb commented 6 years ago

@weotch could you paste me your config and the error you are getting?

henryruhs commented 6 years ago

The good news, it is working on Linux. Congrats!

May I suggest to add a .travis.yml to your repo to integrate Travis CI? I added a basic example for you that contains running npm test on nodejs 6, 7 and 8 plus on Linux an Mac OS:

language: node_js

node_js:
- 6
- 7
- 8

os:
- linux
- osx

install:
- npm install

script:
- npm test
gregnb commented 6 years ago

@redaxmedia That's good news!

Travis was already setup. I went ahead and added the different os's and node versions.

@weotch If you still have issues open up another ticket. Closing this one

weotch commented 6 years ago

@gregnb sounds good. I think my issue may have just been trying to copy a file that didn't actually exist when running webpack dev server.