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

Resolve main field as array for Bower #4

Closed
wants to merge 4 commits into from
Closed

Resolve main field as array for Bower #4

wants to merge 4 commits into from

Conversation

mrm007
Copy link

@mrm007 mrm007 commented Mar 22, 2014

@mrm007 mrm007 changed the title fixes #3 and webpack/webpack#143 Resolve main entry as array for Bower Mar 22, 2014
@mrm007 mrm007 changed the title Resolve main entry as array for Bower Resolve main field as array for Bower Mar 22, 2014
@@ -53,6 +53,9 @@ DirectoryDescriptionFilePlugin.prototype.apply = function(resolver) {
mainModules.push(content[field]);
continue;
}
if (Array.isArray(content[field])) {
mainModules.push(content[field]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mainModules is a string array and you push an array to it.

mainModules = mainModules.concat(content[field]);

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. Should it not work as it is?

@sokra
Copy link
Member

sokra commented Mar 22, 2014

In #3 we check if it's better to not add this behavior to the core, but extract it into a BowerPlugin. This could handle arrays with multiple element. So I won't merge this until we decided what to do. (I tend to extract it into a BowerPlugin)

@mrm007
Copy link
Author

mrm007 commented Mar 22, 2014

This is a small change which

  • is guarded by tests
  • doesn't break any existing functionality
  • enables us to import Bower components easily

So I don't get it -- why the need for BowerPlugin?

@sokra
Copy link
Member

sokra commented Mar 25, 2014

The spec says that all files in the main array should be used, not only the first. This cannot be implemented as patch to enhanced-resolve, but need to be implemented as BowerPlugin (like the ComponentPlugin).

Example: https://github.com/danialfarid/angular-file-upload-bower/blob/master/bower.json do really require both files from the main field.

@mrm007
Copy link
Author

mrm007 commented Mar 25, 2014

I don't think the example you referenced makes sense. From the README file:

<script src="/bower_components/angular/angular-file-upload-shim.js"></script>
<!--only needed if you support upload progress/abort or non HTML5 FormData browsers.-->
<!-- NOTE: angular.file-upload-shim.js MUST BE PLACED BEFORE angular.js-->
<script src="/bower_components/angular/angular.js"></script>
<script src="/bower_components/angular/angular-file-upload.js"></script>

The -shim file is not always needed so I think the package author has made an error including them both in main. See discussions in bower/bower#46 and bower/bower#110.


Futhermore, what if all JS files referenced in main have a default export? i.e.

// a.js
define([], function() {
    return function A() {};
});

// b.js
define([], function() {
    return function B() {};
});

Or in ES6:

// a.js
export default function A() {};

// b.js
export default function B() {};

In these cases how would webpack resolve the bundle?

@sokra
Copy link
Member

sokra commented Mar 26, 2014

@gunta

@timsuchanek
Copy link

Hey, what's the state of this? I really need this for my bower components to work.

@gunta
Copy link

gunta commented Apr 21, 2014

Because the specs for Bower are not finished yet,
first of all, we should create a BowerPlugin, follow (or participate in) the discussion that is going on this new bower spec repository.

For example, main array style might be deprecated, and some WebPack config related features might be added, making a BowerPlugin a must.

@kilokeith
Copy link

DirectoryDescriptionFilePlugin.js cannot parse a the "main" field if it is an array of paths.

"main": [
  "./dist/jquery.countdown.js",
  "./dist/jquery.countdown.min.js"
],

While technically not correct according to the bower.js specs, it doesn't stop people from doing that.

@adri
Copy link

adri commented Oct 9, 2014

+1 for merging this. I'm not using bower but a custom "plugin" definition which states css/js/... files to load. For this it would be very handy to have the array check in place.

@sokra
Copy link
Member

sokra commented Oct 9, 2014

You can do this:

new webpack.ResolverPlugin(
  new webpack.ResolverPlugin.DirectoryDescriptionFilePlugin("bower.json", ["main", ["main", 0]])
)

@adri
Copy link

adri commented Oct 9, 2014

Thanks @sokra. I think I misunderstood what DirectoryDescriptionFilePlugin does. What I actually want is when resolving for example ./plugins/some-plugin, read the plugin.json and load all defined files. Example plugin.json:

{
  "name": "Some plugin",
  "description": "...",
  "css": [
   "css/some-css.css"
  ],
  "js": [
    "js/some-js.js"
  ],
  "templates": [
    "templates/a.html"
  ]
}

I'm having a hard time finding documentation for this, do you have a hint?

@sokra
Copy link
Member

sokra commented Oct 9, 2014

That's not possible by resolving. Resolving means it finds one file for a given request. For your use case you need a loader that generates a artificial module which require all files.

You can combine it with a plugin to resolve it correctly.

That's similar to what the component-webpack-plugin does.

@adri
Copy link

adri commented Oct 9, 2014

Awesome, thank you @sokra

myabc added a commit to opf/openproject that referenced this pull request Nov 12, 2014
n.b. Webpack will not parse the `main` entry of Bower modules when it
is set to an Array:
webpack/enhanced-resolve#4

Signed-off-by: Alex Coles <alex@alexbcoles.com>
@MrJH
Copy link

MrJH commented Mar 12, 2015

Why is this still open since november last year?

@jrunestone
Copy link

@MrJH I don't know but I got away with using this: https://github.com/lpiepiora/bower-webpack-plugin

@sokra sokra closed this Mar 20, 2015
@MrJH
Copy link

MrJH commented Apr 4, 2015

@swemaniac i'll give it a try next week, thx

@faller
Copy link

faller commented May 19, 2015

I tried

new webpack.ResolverPlugin.DirectoryDescriptionFilePlugin("bower.json", ["main", ["main", 0]])

and

new webpack.ResolverPlugin.DirectoryDescriptionFilePlugin("bower.json", ["main"])

but can't solve the problem.

https://github.com/lpiepiora/bower-webpack-plugin help a lot, thx!

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 this pull request may close these issues.

Resolve main entry for bower and component
9 participants