Bug/overrides for token #210
Conversation
|
Hi @develop084! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
facebook-github-bot
added
the
CLA Signed
There was a problem hiding this comment.
I'm not sure this fixes the initial problem, and I think it causes some regressions.
The only change in this PR is that instead of requiring variable media queries to start with @, it can not be any string.
This makes the types less safe, but doesn't fix the initial problem. The actual fix is going to be a bit more complicated then this.
| [Key in keyof Config]: | ||
| | Config[Key] | ||
| | { +default: Config[Key], +[string]: Config[Key] }, | ||
| | { default: Config[Key]; [property: string]: Config[Key] }; |
There was a problem hiding this comment.
Revert this line.
+ means readonly in Flow and is needed.
[string] is valid in Flow, and there is no need for the property: bit.
Please bring back the + here.
There was a problem hiding this comment.
What is the purpose of your PR? Every change this PR breaks Flow by trying to change it to Typescript syntax.
There was a problem hiding this comment.
@nmn I thought the problem occurred , can be fixed by changing the type, it was my first try to contribute to open source I think I am not at the ryt place then.
Thanks
There was a problem hiding this comment.
I will try to fix what's bad in my PR, I am sure that I'll learn something :)
There was a problem hiding this comment.
@develop084 From what I understand, you're trying to change the types of createTheme to allow changing some of the variables instead of all of them.
I've already made this fix and it'll be out in the next release in a few days.
I don't want to discourage you from learning and contributing. I would suggest reading the Contribution.md document, and learn more about Flow at flow.org
Then, make sure you have the Flow plugin for VSCode installed and functional before you make your next attempt.
What changed / motivation ?
The
OverridesForTokenTypetype was updated to allow more flexibility when defining overrides in thestylex.createThemefunction. The property names are no longer restricted to a specific type, providing greater flexibility for direct use of string literals.Linked PR/Issues
Fixes # (replace with the relevant issue number, if applicable)
Additional Context
No additional context is necessary for this change.
Pre-flight checklist
Contribution Guidelines