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

Breaking change of styles' loading order @ 0.6.0 #64

Closed
keann opened this issue Feb 14, 2018 · 4 comments · Fixed by #69
Closed

Breaking change of styles' loading order @ 0.6.0 #64

keann opened this issue Feb 14, 2018 · 4 comments · Fixed by #69

Comments

@keann
Copy link

keann commented Feb 14, 2018

Hi guys.

Let's say there is a following case:

├─ blocks
│ └─ Example
│ │ ├─ _mod1
│ │ │ └─ Example_mod1.css  /* styles #1 */
│ │ ├─ _mod2
│ │ │ └─ Example_mod2.css  /* styles #2 */
│ │ ├─ Example.css         /* styles #0 */
│ │ └─ Example.js
└─ app.js

/*   Example.js   */
...
import 'm:mod1';
export default decl({
  block: 'Example'
});

/*   app.js   */
...
import Example from 'b:Example m:mod2';
...

where mod1 is pretty lightweight and widely used (e.g. some disabled state with one-two loc), thus it's just easier to include it as a base block functionality (but still keep in a separate folder for clarity). Obviously, as mod1 overrides block's base styles, its css should be loaded after the main one.

With loader version <=0.5.1 loading order of styles was predictable & ok: 0 -> 2 -> 1.
But with 0.6.0 now it is 1 -> 0 -> 2 and totally breaks existing logic, described above.

@Yeti-or
Copy link
Member

Yeti-or commented Feb 19, 2018

Valid issue, we will fix it

Yeti-or added a commit to bem-sdk-archive/bem-import-notation that referenced this issue Mar 12, 2018
Previous decision was wrong we actually need to extract main entity
Check this issue: bem/webpack-bem-loader#64
Natural dependencies were broken
Yeti-or added a commit to bem/bem-sdk that referenced this issue Mar 12, 2018
Previous decision was wrong we actually need to extract main entity
Check this issue: bem/webpack-bem-loader#64
Natural dependencies were broken
Yeti-or added a commit that referenced this issue Mar 12, 2018
@Yeti-or
Copy link
Member

Yeti-or commented Mar 13, 2018

@keann check v0.6.1, please

@keann
Copy link
Author

keann commented Mar 14, 2018

@Yeti-or @bem/import-notation@1.1.3 broke everything everywhere)

For example, this code

/* project/blocks/Header/Header.js */
import pt from 'prop-types';
import { decl } from 'bem-react-core';

import 'm:font=roboto|ubuntu';
import 'm:row';
import 'm:size=16|20|24|28|32|40|56';

export default decl(
  {
    block: 'Header',
    ...
  }
);

was previously turned into

before

and now it's

after

where requiring of Header.js from itself results in an object:
(kind of webpack's way of handling recursive require?)

require result

(caught on a breakpoint, and "102" is a webpack's module id of a _Header.js_ script)

So on the 25th line above there is an error of "applyDecls is not a function", because it's, well, undefined.

PS. Webpack version: 3.11.0

@keann
Copy link
Author

keann commented Mar 18, 2018

Thanks a lot for the latest fixes, with 0.6.2 require-chains look much better! For the one who might read this thread in future: as a result the loading order became 0 -> 1 -> 2, what deprives undocumented, yet convenient possibility to make rules in 1 the most important (w/o !important or specificity tricks). But, according to indices, it's the most obvious order, and I totally accept it.

I'm afraid, there is another problem now 😅 Here context of a block seems to be its folder. If we have inside this folder some related test- or preview-files, that condition also prevents from pushing a require of a block itself inside them. As a result it's impossible to import a block anywhere in its folder.

Update
Not sure, which particular update it's about, but the problem above is solved as well by now.

Yeti-or added a commit to bem/bem-sdk that referenced this issue Mar 28, 2018
Previous decision was wrong we actually need to extract main entity
Check this issue: bem/webpack-bem-loader#64
Natural dependencies were broken
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 a pull request may close this issue.

2 participants