Skip to content
This repository has been archived by the owner on Jan 31, 2024. It is now read-only.

Doesn't resolve LESS imports that have no extension #6

Closed
tsullivan opened this issue May 24, 2017 · 11 comments
Closed

Doesn't resolve LESS imports that have no extension #6

tsullivan opened this issue May 24, 2017 · 11 comments

Comments

@tsullivan
Copy link
Member

tsullivan commented May 24, 2017

If an import statement points to a LESS file with no extension, webpack knows how to import it, but the glob pattern builder is specific to JS files.

A workaround is just to add the .less extension in the import, which may actually be a better way to do it, for clarity.

Doesn't work:

import 'helloworld/less/main';

Works:

import 'helloworld/less/main.less';
@w33ble
Copy link
Contributor

w33ble commented May 24, 2017

Do we really have an alias in Kibana that resolves less files without the need for .less?

@tsullivan
Copy link
Member Author

Do we really have an alias in Kibana that resolves less files without the need for .less?

Apparently so. I've been using it, which is how I found this issue 😛

But I'm totally fine with using the workaround for a long-term solution.

@tsullivan tsullivan changed the title Doesn't resolve LESS files Doesn't resolve LESS imports that have no extension May 24, 2017
@tsullivan
Copy link
Member Author

Realized the way I worded the issue title was confusing. I retitled it

@w33ble
Copy link
Contributor

w33ble commented May 25, 2017

@spalger how would you feel about removing that resolution rule in Kibana core, maybe as part of 6.0.0? I don't know if you have an idea of how much code relies on it, I can take a look if you don't know offhand.

@w33ble
Copy link
Contributor

w33ble commented May 25, 2017

@tsullivan is it possible that that import was just undefined, but things worked because you'd already loaded that less file somewhere else, so it was still part of the build? I mean, none of the styles are scoped or anything, it all just ends up in one big bundle.

@rhoboat
Copy link

rhoboat commented May 25, 2017

Just wondering.. we are sure that webpack is importing the less file sans extension?

@tylersmalley
Copy link

tylersmalley commented May 25, 2017

It's due to .less being defined for resolve.extensions https://github.com/elastic/kibana/blob/master/src/optimize/base_optimizer.js#L148

I agree it should be removed. Honestly, the whole setting seems like an anti-pattern.

@spalger
Copy link
Contributor

spalger commented May 26, 2017

I say rip it out and let plugins update to fix it. No need to wait for 6.0. I don't think it's a pattern we should have supported in the first place, and webpack will complain immediately when it can't resolve the file

@w33ble
Copy link
Contributor

w33ble commented May 26, 2017

Are you proposing ripping this out for less files, or in general? I'd like to open an issue upstream (and link back to elastic/kibana#6430, specifically the second point). Removing the less alias specifically seems a lot easier, and I think it's very likely this is used sparingly in core, if at all. The js resolution, in contrast, is everywhere.

@tylersmalley
Copy link

I am proposing removing the resolution for less and possibly jsx. I like following the node standard, which is to resolve js and json.

@spalger
Copy link
Contributor

spalger commented Jun 2, 2017

Fixed by #15

@spalger spalger closed this as completed Jun 2, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants