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

eyeglass import is not consistent with rails/bower/node/etc #1085

Closed
fivetanley opened this issue Sep 29, 2016 · 3 comments
Closed

eyeglass import is not consistent with rails/bower/node/etc #1085

fivetanley opened this issue Sep 29, 2016 · 3 comments

Comments

@fivetanley
Copy link

In #1007, Eyeglass support was added. This was a great improvement for us as it allowed us to pull more dependencies in through npm.

We have a sass library that works both in Rails's asset pipeline and in other ecosystems. The eyeglass import having a different name in Eyeglass unfortunately makes us choose between only supporting building through eyeglass, or not having eyeglass support and having everything else work.

For example, in Rails:

@import "bootstrap";

But in eyeglass:

@import "bootstrap-sass/bootstrap";

If you look at the README in its current state you can see how consistent it is until you get down to the Eyeglass section.

This could be fixed by adding a name field to the eyeglass section package.json for bootstrap-sass:

{
  "name": "bootstrap-sass",
  "version": "1.1.0",
  "eyeglass": {
    "name": "bootstrap",
    "needs": "^0.6.7",
    "exports": "eyeglass-exports.js"
  }

For consistency, I think it would be great if you could @import "bootstrap" in any ecosystem and have it work.

This would be a breaking change for anybody currently using eyeglass, but an easy fix to implement (change your import statements to "@import bootstrap").

@fivetanley
Copy link
Author

@iammikecohen I'd love to hear your input here as you championed the first pull request. There may be a historical reason for the name difference that I'm missing.

@iammikecohen
Copy link

Yes, this should absolutely be done.

@cvrebert
Copy link
Collaborator

cvrebert commented Oct 7, 2016

@glebm I assume this is out of the question due to SemVer?

@glebm glebm closed this as completed Aug 21, 2018
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

No branches or pull requests

4 participants