gbowne1 / codebooker

This is a book recommendation app created with React 18.2 and MUI for coders/programmers looking for reccomendations to books on programming/coding to read
MIT License
31 stars 57 forks source link

FIX : 213 | removed the empty rependency array from useEffect for fetching books #215

Closed hunxjunedo closed 8 months ago

hunxjunedo commented 8 months ago

fixed the issue #213 caused be empty dependency array.

gbowne1 commented 8 months ago

@BlackBond06

Ok, then I think that then we should move to using a useEffect and start making some more optimization.

useEffect(() => {
        const fetchBooksFromDB = async () => {
            try {
                // Fetch data from the database
                const response = await axios.get('http://localhost:3001/api/books/getall');
                const books = response.data;

                // Check if the database has no books
                if (books.length === 0) {
                    const fileResponse = await axios.get('http://localhost:3001/api/books/filedata');
                    const fileBooks = fileResponse.data;

                    // Assuming fileBooks is an array of books from the file
                    setMyRows([...fileBooks]);

                    // Add books to the database
                    await addBooksToDB(fileBooks);
                } else {
                    // Database has books, use them
                    setMyRows([...books]);
                }
            } catch (error) {
                console.error('Error fetching books:', error);
            }
        };

        fetchBooksFromDB(); // Call fetchBooksFromDB only once when the component mounts
    }, []); // Empty dependency array ensures it runs only on mount
thoughts?  It's starting to get a bit slow running.
gbowne1 commented 8 months ago

My other thought was use some Memoization.. ie

const sortedRows = React.useMemo(() => {
    return orderBy(myRows, [sortingColumn], [sortOrder ? 'asc' : 'desc']);
}, [myRows, sortingColumn, sortOrder]);
const filteredRows = React.useMemo(() => {
    return myRows.filter(row => row.title.includes(filter) || row.category.includes(filter) || row.author.includes(filter));
}, [myRows, filter]);

Also to:

gbowne1 commented 8 months ago

A refactored component might look like

Library.jsx

import React, { useEffect, useState, useMemo, useCallback, useRef, Suspense, lazy } from 'react';
import PropTypes from 'prop-types';
import { orderBy, debounce } from 'lodash';
import { Box, Table, TableBody, CircularProgress, TableCell, TableContainer, TableHead, TableRow, Typography, IconButton, TableSortLabel } from '@mui/material';
import { StarIcon, AddIcon, DeleteIcon, MoreHorizIcon, EditIcon, ShareIcon, CloseIcon } from '@mui/icons-material';
import { Button, Modal, TextField, Dialog, DialogActions, DialogContent, DialogContentText, DialogTitle, Snackbar } from '@mui/material';
import Classes from './Library.module.css';
import './Library.css';
import { Rating } from 'react-simple-star-rating';
import axios from 'axios';
import ISBN from 'isbn-validate';
// import { TableVirtuoso } from 'react-virtuoso';
// import rows from './data.json';

function Library() {

    const [books, setBooks] = useState([]);
    const [myRows, setMyRows] = useState([]);
    const [loading, setLoading] = useState(false);
    const [open, setOpen] = useState(false);
    const [data, setData] = useState([]);
    const [error, setError] = useState('');
    const [sortColumn, setSortColumn] = useState('');
    const [sortOrder, setSortOrder] = useState(true);
    const [filter, setFilter] = useState('');
    const filterInputRef = useRef();
    // Fetch books from the database
    useEffect(() => {
        fetchBooksFromDB();
    }, []); // This effect runs once on component mount

    useEffect(() => {
        const fetchData = async () => {
          const response = await fetch('http://localhost:3001/api/books/getall');
          const data = await response.json();
          setData(data);
        };

        fetchData();
    }, []); // This effect runs once on component mount

    // Fetch books from the database
    useEffect(() => {
        const fetchBooks = async () => {
            setLoading(true);
            try {
                const response = await axios.get('http://localhost:3001/api/books/getall');
                setBooks(response.data);
            } catch (err) {
                setError(err.message);
            } finally {
                setLoading(false);
            }
        };

        fetchBooks();
    }, []); // Empty dependency array means this effect runs once on mount

    // Filter books based on the filter state
    useEffect(() => {
        const filteredBooks = books.filter(book => book.title.includes(filter));
        setBooks(filteredBooks);
    }, [filter, books]); // This effect runs whenever filter or books changes

    // Memoize the filter input to prevent unnecessary re-renders
    const handleFilterChange = useCallback((e) => {
        setFilter(e.target.value);
    }, []);

    const handleYearChange = useCallback((event) => {
        // Your logic here
    }, []);

    const handleClose = (event, reason) => {
        if (reason === 'clickaway') {
          return;
        }
        setOpen(false);
    };

    // Memoize the function to fetch books to prevent unnecessary re-renders
    const fetchBooks = useCallback(async () => {
        setLoading(true);
        try {
            const response = await axios.get('http://localhost:3001/api/books/getall');
            setBooks(response.data);
            setMyRows(response.data);
        } catch (err) {
            setError(err.message);
        } finally {
            setLoading(false);
        }
    }, []);

    // Use useRef to access the filter input directly without triggering re-renders
    const focusFilterInput = () => {
        filterInputRef.current.focus();
    };

    const handleSort = useCallback((column) => {
        setSortColumn(column);
        setSortOrder(prevSortOrder => !prevSortOrder); // Toggle sort order
    }, []);

    const sortedData = useMemo(() => {
        let sorted = [...data];
        if (sortColumn) {
          sorted.sort((a, b) => {
            if (a[sortColumn] < b[sortColumn]) {
              return sortOrder ? -1 : 1;
            }
            if (a[sortColumn] > b[sortColumn]) {
              return sortOrder ? 1 : -1;
            }
            return 0;
          });
        }
        return sorted;
     }, [data, sortColumn, sortOrder]);

    // Function to fetch books from the database
    async function fetchBooksFromDB() {
        setLoading(true);
        try {
            const response = await axios.get('http://localhost:3001/api/books/getall');
            setMyRows(response.data);
        } catch (err) {
            setError(err.message);
        } finally {
            setLoading(false);
        }
    }

    // Function to remove a book by its ID
    async function removeBookByName(row) {
        setLoading(true);
        try {
            await axios.delete(`http://localhost:3001/api/books/${row._id}`);
            fetchBooksFromDB(); // Refresh the list of books
        } catch (err) {
            setError(err.message);
        } finally {
            setLoading(false);
        }
    }

    // Function to add a new book
    async function addBook(bookObj) {
        setLoading(true);
        try {
            await axios.post('http://localhost:3001/api/books/newbook', { bookObj });
            fetchBooksFromDB(); // Refresh the list of books
        } catch (err) {
            setError(err.message);
        } finally {
            setLoading(false);
        }
    }

    // Event handler for book review
    function handleBookReview(bookId) {
        // Logic to handle book review
     }

    // Render the component
    return (
        <>
        // Consolidating items into a single Snackbar
        <Snackbar
            action={action}
            open={isDeleted || isAdded || blankEntry || showSnackBar}
            autoHideDuration={3000}
            onClose={closeSnackBar}
            message={isDeleted ? removedItemName : isAdded ? 'Item Added Successfully' : blankEntry ? 'Please enter the required credentials' : bookUploadError}
        />

        {/* Book Review Modal */}
        <Modal
            open={showReviewModal}
            onClose={() => handleReviewModal(false)}
        >
            {/* Modal content */}
        </Modal>

        {/* Reading Reviews Modal */}
        <Dialog
            open={enableReviewModal}
            onClose={() => setEnableReviewModal(false)}
            aria-labelledby='alert-dialog-title'
            aria-describedby='alert-dialog-description'
        >
            {/* Dialog content */}
        </Dialog>

        {/* Add Item Modal */}
        <Modal
            open={showModal}
            onClose={() => handleModalBox(false)}
        >
            {/* Modal content */}
        </Modal>

        {/* Table Component */}
        {myRows.length > 0 && (
            <TableContainer style={{ marginTop: 80, marginInline: 10 }} component={Paper}>
            {/* TableHead and TableBody */}
            </TableContainer>
        )}
        </>
    );
}

Library.propTypes = {
    filter: PropTypes.string,
    setFilter: PropTypes.func,
};
hunxjunedo commented 8 months ago

@BlackBond06 what if we simply introduce a state variable which doesn't ever change throughout the program ? This will also remove ESLint warning and hopefully achieve the purpose.

hunxjunedo commented 8 months ago

Yeah it's reasonable, but still the solution I just suggested will hardly take a few seconds to implement, so I think it's better to remove the warning if it's one step solution.

gbowne1 commented 8 months ago

@LOGESH-B @BlackBond06 etal.. fine, but It's just not being very efficient..

Yeah we could just ignore the warning but at some point it needs to be addressed unless we stick in a thing that tells eslint to ignore it.

This file is bordering 1k lines.. and yeah I think it needs some refactorization.. and I haven't really been that happy with the whole view overall.

the first 25 lines of Library.jsx as we have it now, are not needed as they can be all imported in one.

I may do some work and refactor the component with:

gbowne1 commented 8 months ago

I am going to merge this with the suggestion that we work ob refactoring the Library.jsx file

hunxjunedo commented 8 months ago

The other tasks may take time, but I think i should pass the empty dependecy array as despite giving eslint warning, its the best solution, as it doesn't call the function whenever anything renders, as mentioned in the first comment by @BlackBond06