-
Notifications
You must be signed in to change notification settings - Fork 571
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
feat: refactor reference utils and expose as standalone functions #1056
Conversation
* @param {Object} opts | ||
* @returns {string} | ||
*/ | ||
export function resolveReferences( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactored "compile_value" function that used to be inside resolveObject file
@@ -0,0 +1,223 @@ | |||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly copied from the resolveObject tests but I refactored every single test to make it a unit-test for the resolveReference function, which is now more granularly tested
@@ -11,7 +11,7 @@ | |||
* and limitations under the License. | |||
*/ | |||
import { expect } from 'chai'; | |||
import resolveReference from '../../../lib/utils/references/resolveReference.js'; | |||
import getValueByPath from '../../../lib/utils/references/getValueByPath.js'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needed a rename imo, bit confusing, it doesn't actually resolve the reference, just gives you the value of the token using the token "path"
@@ -1,50 +1,61 @@ | |||
export default `<?xml version="1.0" encoding="UTF-8"?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this file now imports from style-dictionary/utils
I was getting errors about "import not allowed outside module" error or whatever. I couldn't find the culprit, I imagine it has something to do with lodash "template" util not allowing ESM imports or something, so I just went ahead and refactored this thing to use native ES6 Template Literals instead, as I'm planning to do with all templates (so we can ditch lodash)
Description of changes:
Makes reference utility functions not bound to the dictionary instance.
This makes it easier to expose them as publicly usable utilities, makes them more straightforward to use by users (no need to do
const boundGetReferences = getReferences.bind({ properties: ... });
in order to use)By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.