Skip to content

Conversation

@pusherman
Copy link

Adds support for lazy importing of rows to handle very large import files quickly and memory efficiently

Copilot AI review requested due to automatic review settings August 17, 2025 13:57
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds lazy importing functionality to handle very large import files efficiently by using Laravel's LazyCollection instead of loading all data into memory at once.

  • Introduces importLazy() method that returns a LazyCollection for memory-efficient processing
  • Refactors existing import logic into reusable methods to support both eager and lazy loading
  • Adds comprehensive test coverage for the new lazy import functionality

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
tests/LazyImportTest.php Adds test cases for lazy import functionality with and without callback mapping
src/Importable.php Implements lazy import method, refactors row normalization logic, and adds generator-based sheet processing

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

}
if ($this->transpose) {
// Fallback to non-lazy processing when transposing
throw new \Exception('Transposing is not supported with lazy import.');
Copy link

Copilot AI Aug 17, 2025

Choose a reason for hiding this comment

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

The error message should be more descriptive and use a specific exception type. Consider using InvalidArgumentException with a message like 'Transpose functionality is not compatible with lazy importing. Use import() method instead.'

Suggested change
throw new \Exception('Transposing is not supported with lazy import.');
// Transpose functionality is not compatible with lazy importing
throw new \InvalidArgumentException('Transpose functionality is not compatible with lazy importing. Use import() method instead.');

Copilot uses AI. Check for mistakes.
$cell instanceof Cell\FormulaCell => $cell->getComputedValue(),
default => $cell->getValue(),
};
}, $rowAsObject->getCells());
Copy link

Copilot AI Aug 17, 2025

Choose a reason for hiding this comment

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

The cell processing logic is duplicated between importSheet() and importSheetGenerator(). Consider extracting this into a private method like processCellValue(Cell $cell) to reduce code duplication.

Suggested change
}, $rowAsObject->getCells());
$row = array_map([$this, 'processCellValue'], $rowAsObject->getCells());

Copilot uses AI. Check for mistakes.
* @param array $headers
* @param int $count_header
*
* @return array|null
Copy link

Copilot AI Aug 17, 2025

Choose a reason for hiding this comment

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

The normalizeRow method lacks proper PHPDoc documentation. It should include @param and @return annotations with type information and descriptions for the reference parameters.

Suggested change
* @return array|null
* @param int $k The current row index.
* @param array $row The row data to normalize.
* @param array &$headers Reference to the headers array, updated when header row is encountered.
* @param int &$count_header Reference to the header count, updated when header row is encountered.
*
* @return array|null Returns an associative array if headers exist, the raw row if not, or null to skip processing.

Copilot uses AI. Check for mistakes.
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.

1 participant