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

fix(export platform): fixing infinite loop when there are reference errors #531

Merged
merged 2 commits into from
Feb 2, 2021

Conversation

dbanksdesign
Copy link
Member

@dbanksdesign dbanksdesign commented Feb 2, 2021

Issue #, if available: #524

Description of changes: When there were property reference errors (could not find reference or circular reference) Style Dictionary would get caught in an infinite loop. This fixes that by changing the logic of when loop within exportPlatform finishes by checking if it performed any transformations. This is because we keep the reference in the dictionary object when it is erroneous so it would keep thinking there is a deferred transform.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

const transformed = transformObject(exportableResult, platformConfig, transformationContext);

// referenced values, that have not (yet) been transformed should be excluded from resolving
const ignorePathsToResolve = deferredPropValueTransforms.map(p => getName([p, 'value']));
exportableResult = resolveObject(transformed, {ignorePaths: ignorePathsToResolve});

console.log(transformationsCountBefore, transformedPropRefs.length, deferredPropValueTransforms.length);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was an accidental include of some debug code?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right you are. This is why I need my code to be reviewed.

Comment on lines 104 to 111
if (
// if there is no deferred value transforms left we are done
deferredPropValueTransforms.length === 0 ||
// or if we didn't transform anything since the last loop.
// This would happen if a token reference is not found or is circular
// because we keep the broken reference
transformedPropRefs.length === transformationsCountBefore
) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I trust your logic, and it makes sense... but it seems unintuitive to me that this is a solid "or".

@chazzmoney
Copy link
Collaborator

Do you think we need to add something to our release notes about the change in output if you have a broken reference?

Otherwise, LGTM! :shipit:

Copy link
Collaborator

@chazzmoney chazzmoney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work.

@dbanksdesign dbanksdesign merged commit 6078c80 into 3.0 Feb 2, 2021
@dbanksdesign dbanksdesign deleted the fix-524 branch February 2, 2021 23:56
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.

2 participants