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

Export locals if they are available and not on .use() #128

Merged
merged 1 commit into from Mar 6, 2017

Conversation

cusspvz
Copy link
Contributor

@cusspvz cusspvz commented Jun 1, 2016

style/useable should match the usual expected behavior and export locals even if the styles aren't being used.

On SSR, use will throw an error, and you will never be able to access locals.
Please give priority to this. Thanks.

@NekR
Copy link

NekR commented Jun 26, 2016

+1 to this. Another way (if you don't want expose locals immediately) could be this:

exports.use = function() {
  // ...
  return content.locals;
}

@cusspvz
Copy link
Contributor Author

cusspvz commented Jun 27, 2016

@NekR I think you should have always access to locals, because that's the default behavior on others. My on my use case, I'm using React and created a React decorator that calls .use() and .unuse() whenever a component is mounted or unmounted. So far so good, until I tried to do it Isomorphically and on the server-side, styles are never used, which means it doesn't have locals and it doesn't attribute the right classes on the render.

I had to change the style-loader by hand and include it on my project until this is merged.

@cusspvz
Copy link
Contributor Author

cusspvz commented Jun 27, 2016

ping @sokra

@NekR
Copy link

NekR commented Jun 27, 2016

@cusspvz Yeah, I agree. I am currently using your solution too. Hope it'll be merged.

@cusspvz
Copy link
Contributor Author

cusspvz commented Jan 29, 2017

I'm releasing a boilerplate that needs this fix.
Any chance of have it merged?

Also, tried to sign cla license and it seems this repo is not registered yet.
captura de ecra 2017-01-29 as 19 36 10

@SpaceK33z SpaceK33z closed this Jan 30, 2017
@SpaceK33z SpaceK33z reopened this Jan 30, 2017
@SpaceK33z
Copy link
Contributor

@cusspvz we're having some issues regarding the CLA at the moment, it probably doesn't like that we moved this repo from webpack to webpack-contrib. You can sign the CLA here: https://cla.js.foundation/webpack/webpack. Sorry for the troubles.

@cusspvz
Copy link
Contributor Author

cusspvz commented Jan 30, 2017

@SpaceK33z done!

Yeah, I've also tried with webpack-contrib instead before but I wasn't successful. Thanks.

@joshwiens
Copy link
Member

@cusspvz - There were compliance issues we needed to clear up before the CLA bot could be enabled, that was cleared up about 10 days ago.

To get the CLA signed, you just need to close this PR & open it again. That will trigger the CLA workflow again which should allow you to sign it.

Please note, before you close & open it again ensure the email in your local git config matches what you have configured in github.

@michael-ciniawsky michael-ciniawsky removed their request for review March 6, 2017 17:43
@cusspvz cusspvz closed this Mar 6, 2017
@cusspvz cusspvz reopened this Mar 6, 2017
@jsf-clabot
Copy link

jsf-clabot commented Mar 6, 2017

CLA assistant check
All committers have signed the CLA.

@cusspvz
Copy link
Contributor Author

cusspvz commented Mar 6, 2017

Done!

@michael-ciniawsky michael-ciniawsky merged commit 92c2279 into webpack-contrib:master Mar 6, 2017
@michael-ciniawsky
Copy link
Member

@cusspvz Thx 👍

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 this pull request may close these issues.

None yet

7 participants