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

Feature Request: Exporting values. #396

Closed
AndrewLeedham opened this issue Feb 7, 2018 · 4 comments

Comments

Projects
None yet
2 participants
@AndrewLeedham
Copy link
Contributor

commented Feb 7, 2018

It would be nice to reference @value variables in JavaScript similar to how CSS Modules handles this.

This would primarily be useful when setting default SVG colours for a React Icon system based on stored CSS colour values, as all the colours would be defined once in the CSS reducing duplicated code.

@tivac

This comment has been minimized.

Copy link
Owner

commented Feb 7, 2018

This seems totally reasonable to me & I have a confession: I assumed that was already happening for some reason 😞

file.exports = message(result, "classes");

Would need to be changed to:

file.exports = Object.assign(
    Object.create(null),
    message(result, "values"),
    message(result, "classes")
);

⚠️ Any @values that happen to have the same name as a class would be stomped (in order to preserve back-compat I think classes have to take priority over @values). ⚠️

Thoughts?

@AndrewLeedham

This comment has been minimized.

Copy link
Contributor Author

commented Feb 7, 2018

I agree classes should definitely take priority. However would it be possible to "namespace" the variables, and add them to a names object so the following would occur:

@value main: #FF00FF;

.main{
    background: main;
}

Would export:

{
    "main": "j21b4ih__main",
    "values": {
        "main": "#FF00FF"
    }
}
@tivac

This comment has been minimized.

Copy link
Owner

commented Feb 7, 2018

I agree that's a nicer way to avoid collisions, but it's probably better to match CSS Modules in this case. I haven't tested it but based on the documentation it seems like their approach is also prone to collisions.

I should get CSSM set back up to test that assumption, I wonder if it's less broken on Windows yet...

AndrewLeedham added a commit to AndrewLeedham/modular-css that referenced this issue Feb 8, 2018

feat: Exporting values. (tivac#396)
Added defined values to the exported JSON object.

AndrewLeedham added a commit to AndrewLeedham/modular-css that referenced this issue Feb 8, 2018

@AndrewLeedham

This comment has been minimized.

Copy link
Contributor Author

commented Feb 8, 2018

I had a dig around your codebase. I managed to get values exporting correctly, however those stored in the values attribute from post-css messages seem to be coming from values that are imported with import * from ./example.css not the actual value definitions themselves. So I used file.values instead which seemed to store defined values in the same structure.

I'm not particularly familiar with either post-css or your codebase, so there may be some unforeseen issues here, but I thought I'd get it working locally so I can start implementing the syntax in my current project and submit a PR in case I was on to something.

@tivac tivac closed this in 479866d Feb 8, 2018

tivac added a commit that referenced this issue Feb 9, 2018

feat: Exporting values. (#396) (#398)
BREAKING CHANGE: Values will now be exported alongside composed classes

Fixes #396
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.