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

Support of @imports from within SCSS files #46

Closed
alex35mil opened this issue Jul 31, 2017 · 6 comments
Closed

Support of @imports from within SCSS files #46

alex35mil opened this issue Jul 31, 2017 · 6 comments

Comments

@alex35mil
Copy link
Member

I see a number of issues raised due to added support of @imports from within SCSS files. This is a non-recommended way of using this loader (mainly for performance reasons). We have 2 opened PRs w/ fixes (#42, #45), these will be merged as soon as they are ready. If these PRs won't solve all issues related to such @imports, I incline to remove this feature completely. Meanwhile, can users of this feature provide use-case(s) when this feature is required? (b/c I can't think of any)

@IAMtheIAM
Copy link

IAMtheIAM commented Aug 7, 2017

Use case: I use @import like this

Root level app.style.scss

@import '../../assets/styles/global';

global.scss

@import "global/susy-grid-config";
@import "global/reset";
@import "global/forms";
@import "global/alignments";
@import "global/display";
@import "global/buttons";

I thought that is pretty standard for writing scss?

@alex35mil
Copy link
Member Author

I thought that is pretty standard for writing scss?

@IAMtheIAM Yep, but it causes performance issues and regressions, so I'm trying to be pragmatic here: if it's faster and safer to list these files in a webpack config, then this is the way to go.

All in all, there are few options here:

  1. Things might be fixed in 1.3.1 so everyone is happy 🤞
  2. Things are still broken and you can fix them and maintain this functionality.
  3. Things are still broken and we remove this functionality, so the listing of resources must be moved to webpack config (no cons that matter).

P.S. Replace global.scss w/ global.js which exports an array of resolved paths to resources to keep your structure untouched (if this is your concern).

@IAMtheIAM
Copy link

@alexfedoseev Crossing fingers for solution 1.

Can you explain solution 3 better? For example, how does global.js feed the scss into webpack? So I export an array of resolved paths to .scss files, but then now it seems I'll need some new loader to process that file in webpack since its a .js file now. Any more tips would help so I can prepare if that needs to happen. Thanks

@alex35mil
Copy link
Member Author

Can you explain solution 3 better? For example, how does global.js feed the scss into webpack? So I export an array of resolved paths to .scss files, but then now it seems I'll need some new loader to process that file in webpack since its a .js file now. Any more tips would help so I can prepare if that needs to happen. Thanks

@IAMtheIAM For example, there's SCSS file w/ resources index at path path/to/resources/index.scss:

@import "./colors";
@import "./sizes";
...

In webpack config loader is defined like this:

{
  loader: 'sass-resources-loader',
  options: {
    resources: './path/to/resources/index.scss',
  }
}

Instead of using SCSS imports you can do the following:

path/to/resources/index.js

import path from 'path';

const resources = [
  'colors.scss',
  'sizes.scss',
  ...
];

export default resources.map(file => path.resolve(__dirname, file));

webpack.config.js

import sassResources from './path/to/resources';
 ...

{
  loader: 'sass-resources-loader',
  options: {
    resources: sassResources,
}

@IAMtheIAM
Copy link

IAMtheIAM commented Oct 12, 2017

I see, thanks, that does make sense. I would still put that at last resort, because it loses the ability to use "navigate to code" in the IDE and quickly switch to the scss file. But if this is the only way, then it will do.

So every scss import because a sass resource now instead, which would make all of them globally available to every other scss file. Correct?

That would only become a problem is you only wanted some imports affecting certain scss files, in the case where having it globally would conflict with other css definitions. I guess one could work around that by very carefully considering the css specificity of their definitions. I am just thinking out loud here.

@IAMtheIAM
Copy link

Good news for all. Version 1.3.1 fixed the issue! My @import statements now are resolve with no problem. Thanks for fixing this @alexfedoseev! This issue can be closed.

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

3 participants