Vue Star Rating Component

Question

I am wondering if someone can do a code re"vue" specifically for my star rating component ([complete code available on github).

I am looking for feedback on best practices, any glaring code crimes I might have committed and bad code.

The code is also deployed to #Heroku here

Problems I saw with the code previous code are also included.

1. The partial star sometimes just gets the default colors, even if the DOM has the colors assigned via props.
2. Change in data (using Vue Dev Tools) does not re-render my star rating component. For the benefit for the code reviewers, I have hosted the component on Heroku, so if you have the Vue dev tools, you can test this too.

Edit - Inlined the code to follow the code-review standards.

Edit - 10/23 - I have updated the code since I first posted this and seem to have resolved all the issues with my previous code. The link to Github is here. Also the inlined code is now newer... I have added comments in the code, where I would love some feedback and comments.

    <template>
    <div>
        <!-- Is this use of dynamic component correct? -->
        <component v-for="n of this.totalStarsData" :key="n" :is="currentComponent(n)" :fillColor="ratingData - n > -1 ? fillColorData : baseColorData" :baseColor="baseColorData" :rating="ratingData"></component>
    </div>
    </template>

    <script>
// Can Star and PartialStar components be dynamically imported?
import Star from "./Star"
import PartialStar from "./PartialStar"

// Using a global variable like this - Good or bad?
let isPartialRendered = false
export default {
    name: 'star-rating',
    components: {
        Star,
        PartialStar
    },
    props: {
        totalStars: {
            type: Number,
            default: 5
        },
        rating: {
            type: Number,
            required: true
        },
        fillColor: {
            type: String,
            default: '#C00'
        },
        baseColor: {
            type: String,
            default: '#666'
        },
    },
    data(){
        return {
            // Any better way of converting props to data?
            // Also maybe naming convention best practices?
            totalStarsData: this.totalStars,
            ratingData: this.rating,
            fillColorData: this.fillColor,
            baseColorData: this.baseColor,
        }
    }, 
    methods: {
        currentComponent: function (count) {
            const int_part = Math.trunc(this.ratingData);
            if(count > int_part && isPartialRendered === false) {
                isPartialRendered = true;
                // Re-setting the isPartialRendered flag here, for cases where the Partial SVG is the last SVG
                if(count === this.totalStarsData){
                    isPartialRendered = false;
                }
                return 'PartialStar'
            } else {
                if(count === this.totalStarsData){
                    isPartialRendered = false;
                }
                return 'Star'
            }

        }
    }
}
</script>

<style scoped>

</style>

Answer 1

Converting props to data is a bad idea if it is not a copy of the prop because you might end up in a situation where your parent changes your prop and your component changes your data and then which is right? The component manipulating props is not good. Instead decide which is externally sourced (from parent) and which is internally manipulated, and then assign each to either prop or data.

Since your code in particular doesn't actually manipulate the data or props you are simply better off if you remove the data key completely, just leave it with props and you can access the values in precisely the same way using this.totalStarsData for example. You can clearly see that having a props key and manually entering the same info into data is ugly code anyway.

Having that global variable is unhelpful in my opinion, and probably bad practice since it goes against the decoupling of the component. Why not make it another prop with a default value of false?