0

update Ok, so I've noticed that even though in isCategoryActive() function I'm mutating only the variable newCategories that was assigned a value from this.props.searchCategories the searchCategories prop changes value as well, therefore passing it to the consecutive array item's invocation of the isCategoryActive function. Why is it happening though??update

I'm building a blog's frontend in React based on Wordpress REST API and I'm having problems creating links to filter posts categories after checking if they are already filtered. The problem I'm having is that even though I wrote a pure function isCategoryActive inside the map function every consecutive category link url has every preceding category id in it. I would have thought that on every invocation of a pure function I would receive a clean result, but in my case it isn't like that. At the moment the wordpress stores 3 categories: "uncategorized" with id: 1, "javascript" with id: 4, "third category" with id: 10

What I'm trying to get the console.log(newCategories, url) function inside the render() function to log:

[1] blog?categories=1
[4] blog?categories=4
[10] blog?categories=10

But at the moment it logs:

[1] blog?categories=1
[1,4] blog?categories=1,4
[1,4,10] blog?categories=1,4,10

I have no idea why it's keeping the record of the previous categories ids.

Here's the code:

// import dependencies
import React, { Component } from 'react'
import { Link } from 'react-router-dom'
import axios from 'axios'

// import components
import '../Styles/Css/PostsCategories.css'

import { createSearchUrl } from './SharedModules'

class PostsCategories extends Component {
  constructor() {
    super()
    this.state = {
      categories: null,
      loading: false
    }
  }

  componentDidMount() {
    this.setState({
      loading: true
    })
    axios.get(`http://localhost/wordpress-api/wp-json/wp/v2/categories`)
      .then(res => {
        this.setState({
          categories: res.data,
          loading: false
        })
      })
  }

  isCategoryActive = (category) => {
    let newCategories = this.props.searchCategories
    newCategories.indexOf(category) === -1
    ? newCategories.push(category)
    : newCategories.splice(newCategories.indexOf(category),1)
    return newCategories
  }

  render() {
    if (this.state.loading || !this.state.categories) return <div className='posts-categories'><h2 className='loading'>Loading ...</h2></div>

    const categories = this.state.categories.map(category => {
      const newCategories = this.isCategoryActive(category.id)
      const url = createSearchUrl('/blog', newCategories, this.props.searchString)
      console.log(newCategories, url)
      return (
        <Link
          to={url}
          onClick={this.props.searchCategoryChange.bind(this, category.id)}
          className='posts-category'
          key={category.id} >
            {category.name}
        </Link>
      )})

    return (
      <div className='posts-categories'>
        {categories}
      </div>
    )
  }
}

export default PostsCategories
Mateusz Mysiak
  • 548
  • 6
  • 19

4 Answers4

1

Inside your original function there's this:

let newCategories = this.props.searchCategories

This won't actually copy searchCategories but instead reference it. They're both pointing at the same array, which is why the original searchCategories array is modified as well.

When you map over the searchCategories array (like you do in yor own solution) you are building a new array (with the push statement) without modifying the searchCategories array. This is however a very convoluted way of making a copy of the array.

jpuntd
  • 902
  • 6
  • 9
  • Great to know @shubham and snwclone . Thanks. Another question from me would be, why is it referencing the prop instead of assigning value? Does it do it because of we're operating within a class and it changes it? Would You care to share some information as to when a value is referencing another and when it gets assigned its value? – Mateusz Mysiak Jul 16 '17 at 15:31
  • It has nothing to do with operating within a class. The best explanation I can find is [this SO question](https://stackoverflow.com/questions/6605640/javascript-by-reference-vs-by-value) – jpuntd Jul 16 '17 at 16:19
  • Ok. I get it now. Thanks very much. Weird that I never had such a problem before. – Mateusz Mysiak Jul 16 '17 at 16:23
1

The problem is that you are mutating this.props.searchCategories directly since when you assign

let newCategories = this.props.searchCategories

it is assigned by reference

You need to have create a new object rather than directly assigning it. For this you can make use of spread operator rather than mapping over the props array and pushing the items to newCategories array.

isCategoryActive = (category) => {
    let newCategories = [..this.props.searchCategories]
    newCategories.indexOf(category) === -1
    ? newCategories.push(category)
    : newCategories.splice(newCategories.indexOf(category),1)
    return newCategories
  }
Shubham Khatri
  • 270,417
  • 55
  • 406
  • 400
  • Javascript assignment is done by reference and so when you do `let newCategories = this.props.searchCategories` thus you are assigning the props by reference and modifying newCategories modifies prop searchCategories – Shubham Khatri Jul 16 '17 at 15:36
1

Instead of

let newCategories = this.props.searchCategories

You could write as

let newCategories = this.props.searchCategories.splice();

Which will create a new Object.

JavaScript by default only copy the reference when we directly assigning Objects (as well as arrays). That's that when the new Object changes, Original also get changed.

You have to use either map, splice or Object.assign to create a new Object.

Lakshitha Ruwan
  • 949
  • 1
  • 9
  • 12
0

Based on the new information from the update that I've discovered I've modified the isCategoryActive function to this:

isCategoryActive = (category) => {
    let newCategories = []
    this.props.searchCategories.map(category => newCategories.push(category))
    newCategories.indexOf(category) === -1
    ? newCategories.push(category)
    : newCategories.splice(newCategories.indexOf(category),1)
    return newCategories
  }

And now it's working perfectly. However, I still don't fully grasp why it was storing the record of previous ones before. I suppose I missed something really silly, but it would be nice if anyone would describe to me what fundamentally went wrong in the first place.

Mateusz Mysiak
  • 548
  • 6
  • 19