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

allow overriding files with a relative @import #406

Closed
azul opened this issue Jul 25, 2017 · 1 comment
Closed

allow overriding files with a relative @import #406

azul opened this issue Jul 25, 2017 · 1 comment

Comments

@azul
Copy link

azul commented Jul 25, 2017

I have a simple app/assets/stylesheets/application.scss:

@import 'head'
...

head.scss contains the defaults but is meant to be overwritten from a customizable directory that is preceedign app/assets/stylesheets in the asset load path.
But since head.scss is found in the path relative to application.scss the head.scss will never be used.

I can work around this by either putting the application.scss in the customization path or moving everything into a subdir so i can use paths that can only be read as absolute paths.

Both workarounds are counterintuitive and should not be needed. If i @import sub/relative from within /foo/parent.scss I would expect sass-rails to look for 'foo/sub/relative' inside the load_paths according to their priority and if no such file is found to look for 'sub/relative'.

This way one could overwrite any file by putting another file in the same location in a load path with higher priority.

@azul azul changed the title allow overriding files with a relative path @import allow overriding files with a relative @import Jul 25, 2017
@azul
Copy link
Author

azul commented Jul 25, 2017

@rafaelfranca was cited on http://github.com/spree/spree/issues/3415#issuecomment-32821148

I think the proper fix is to put the current path to the end of the search.

I'm proposing something else here because i think defaulting to relative paths is sass's way of handling the import ambiguity and we should not break that.

leap-code-o-matic pushed a commit to leapcode/leap_web that referenced this issue Jul 26, 2017
fixes #8794

Reported the underlying issue here:
rails/sass-rails#406

Basically `@import` works like this:
* look for the file relative to the current file
* look for the file as an absolute path following the priorities in the
* asset load_paths

If the file can be imported as a relative path that will take
precedence.

So in order to pick up the head and tails inside customization rather
than in app/assets there are three possibilities:
1) use an absolute path. This is not as easy as it seems. There is no
way of indicating a path is meant to be absolute so we would have to
ensure it does not resolve to a relative path.
2) have a application.scss file inside the customization folder. Since
this is the main file it will be used instead of the app/assets one. In
there relative paths will now also default to the customization folder
rather than app/assets. Once we are in an app/assets file though it will
not go back to picking up customization with relative paths
3) use //= require instead of import. rails-sass advices against this as
each required file would be compiled on it's own and variables could not
be shared.

Going with option 1 here:
```scss
// application.scss:
@import "custom/head_import";
```

```scss
// custom/head_import.scss:
@import "head";
```

As long as there is no custom/head.scss in app/assets it will import
head as an absolute path and thus prefer config/custom over app/assets.

This seems like the best option for now as it does not require changes
to the deployments.
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

2 participants