taptapship / wiredep

Wire Bower dependencies to your source code.
MIT License
1.15k stars 142 forks source link

Cannot inject file with full path containing ignorePath anywhere #194

Closed joostvunderink closed 9 years ago

joostvunderink commented 9 years ago

There can be an undesired interaction between the --ignorePath option and the full filename of the js file to inject, which leads to incorrect filenames in the injected HTML.

A concrete example. We are using the bower module angular-jsonrpc-client. The main file, as specified in bower.json, is src/angular-jsonrpc-client.js. Therefore, the injected file should be:

bower_components/angular-jsonrpc-client/src/angular-jsonrpc-client.js

However, this is injected:

bower_components/angular-jsonrpc-src/angular-jsonrpc-client.js

When you look closely, you see that client/ has been removed from the full path. Our --ignorePath is also client/.

This piece of code from lib/inject-dependencies.js causes this behaviour:

    dependencies.
      map(function (filePath) {
        return $.path.join(
          $.path.relative($.path.dirname(file), $.path.dirname(filePath)),
          $.path.basename(filePath)
        ).replace(/\\/g, '/').replace(ignorePath, '');
      })

It's replace(ignorePath, '') that's the culprit. With ignorePath="client/", this removes client/ anywhere from the full path. I can't imagine that this is the correct behaviour, because the full path of the main js file from any module could contain basically any arbitrary sub-path and would be at risk of this substitution.

Because I'm not exactly sure what the exact use is of ignorePath, I am not sure what the best solution is. A possible solution would be:

.replace(new RegExp('^' + ignorePath), '')

This would only remove the ignorePath from the start of the full path. My guess is, that this is the desired behaviour.

If anyone can confirm that this is a good solution, I'd be happy to create a pull request.

Thank you for your time.

P.s.: I think this issue might be related: https://github.com/taptapship/wiredep/issues/86

stephenplusplus commented 9 years ago

Thanks for the detailed report! I wish every issue opened looked like this :)

ignorePath is mostly for this case:

File tree:

/bower_components
  /jquery
    /jquery.js
/public
  /index.html
bower.json

What will be injected:

<script src="../bower_components/jquery/jquery.js"></script>

With ignorePath "../":

<script src="bower_components/jquery/jquery.js"></script>

We've found it's best to default to mapping file locations from a bower package's file to the user's source file. ignorePath is an option that lets you have a quick way to snip out what you don't need from that path.

We also have (as an example) config.fileTypes.html.js: function(path) { return newPath; } that would give you complete control of the path that gets injected into a js block within an HTML source file.

For this issue, you can change ignorePath: "client/" in your config to a RegExp that better suits your situation (maybe ignorePath: /^client/), but changing its behavior could wreck a lot of projects.

joostvunderink commented 9 years ago

Thank you for your reply. I have followed your suggestion of ignorePath: /^client/ and it worked well.

It might be worth it to think about somehow warning the user, who is depending on wiredep, that their ignorePath has just eaten a bit of the full path somewhere in the middle.

As far as I'm concerned, this issue can be closed.