feat(atomic): string serialization of atoms #934
Conversation
|
Hi @Anber , thanks so much for looking at the other PR's I've put up! This one addresses some issues we've come across when using atomic styles in Linaria – namely, it's confusing for people that Would love to get your thoughts! The issue linked provides more detailed rationale. Thanks again! |
packages/babel/tsconfig.json
Outdated
| @@ -2,6 +2,7 @@ | |||
| "extends": "../../tsconfig.json", | |||
| "compilerOptions": { "paths": {}, "rootDir": "src/" }, | |||
| "references": [ | |||
| { "path": "../atomic" }, | |||
There was a problem hiding this comment.
Looks like it's unneeded here.
There was a problem hiding this comment.
I realized this is being added because there's a dev dependency @linaria/atomic. We can keep it removed it but I think that's why it's being re-added.
|
Hi @jpnelson! |
|
@Anber I added another commit to this PR, to add property priorities to address #902 . It depends on this PR so I added it to this one. Do you think you'd be able to do a release when this goes in? (I think we'd need new versions of |
|
@Anber sorry to ping again on this – do you think a merge / release is possible? We're also testing and iterating on these features in our project. I'm hopeful this is the last major change, but if the frequency of releases is inconvenient, we could create a fork for our project and try and contribute back later in a larger chunk. Thanks again for your time! 🙏 |
|
@jpnelson sorry for the delay, I was in mountains for the last two weeks almost without all communications. I'll merge it and release in a couple of days. |
|
Haha that's very understandable, thank you! 🙏 |
| @@ -1,6 +1,26 @@ | |||
| import postcss, { Document, AtRule, Container, Rule } from 'postcss'; | |||
| import { slugify } from '@linaria/utils'; | |||
| import stylis from 'stylis'; | |||
| import { all as knownProperties } from 'known-css-properties'; | |||
| import { getPropertyPriority } from './propertyPriority'; | |||
There was a problem hiding this comment.
Hi @jpnelson
Looks like you didn't add this file to the PR
There was a problem hiding this comment.
my mistake, sorry @Anber ! I've added it back in #950 . I think this was missing because it was originally a separate PR.
Sorry I missed that too – the precommit hooks seem extensive, but let me know if there's anything else I can run before merging – CI doesn't seem to start for me when I create PR's.
Motivation
Addresses #912
Summary
Should be fairly straightforward after reading the reason why we might want this. I've also updated the documentation to reflect what's going.
Test plan
Tested with snapshot tests and unit tests. Docs should render the same way