tiltshift / figmint

sync Figma styles to JS
MIT License
97 stars 7 forks source link

Fix issue where figmint outputs invalid js #27

Closed bibixx closed 4 years ago

bibixx commented 4 years ago

Issue

When importing a significant number of elements (>100) the util.inspect doesn't behave properly ā€“ instead of outputting the objects it returns ... 1 more item text.

Example

const styles = {
  fillStyles: [
    {
      key: 'KEY_1',
      name: 'RED',
      styles: [
        {
          type: 'SOLID',
          blendMode: 'NORMAL',
          color: 'hsla(0, 100%, 50%, 1)'
        }
      ]
    },
    // here lays 99 more fill styles
    {
      key: 'KEY_100',
      name: 'Generic / White',
      styles: [
        {
          type: 'SOLID',
          blendMode: 'NORMAL',
          color: 'hsl(0, 0%, 100%)'
        }
      ]
    },
    ... 1 more item
  ],
  textStyles: [],
  effectStyles: [],
  gridStyles: [],
  exports: []
}

export default styles

Please note that I am using the old output format here, as it is what I am using in my project. However the same issue happens on the newest version of figmint

Concerns

Was there any specific reason to using util.inspect instead of JSON.stringify? Especially as the nodejs documentation states that

The output of util.inspect may change at any time and should not be depended upon programmatically.

chrisdrackett commented 4 years ago

I'm actually not sure why I used this, its been a while and on most other projects I also use strigify. I feel like it had something to do with object depth, but I'm really not sure.

Thanks so much for the PR!

chrisdrackett commented 4 years ago

ah yes, in theory:

When having heavy nested objects, JSON.stringify does not stringify all levels. Would it be possible to use util.inspect instead of JSON.stringify?

chrisdrackett commented 4 years ago

@bibixx I'm still using util.inspect as I don't have time to make sure right now that JSON.stringify does not result in lost data in some situations (I moved to it for a reason). But I've updated it so it should process more than 100 items now. Thanks for the report!

bibixx commented 4 years ago

@chrisdrackett sure, whatever it is to make it work šŸ˜