Contrib.social

Undeclared Dependency on PhpOffice\PhpSpreadsheet

Open on Drupal.org →
Created on 12 June 2023, over 1 year ago
Updated 12 July 2023, over 1 year ago

Problem/Motivation

The PHPStan static analysis tool is reporting several errors in the Views Data Export module related to the PhpOffice\PhpSpreadsheet library. This library doesn't appear to be declared as a dependency in the module's composer.json file, but the module's code seems to be using classes and methods from this library. This could potentially lead to runtime errors if the library isn't installed in the project where the module is used.

Steps to reproduce

1. Install the Views Data Export module in a Drupal 9 project without the PhpOffice\PhpSpreadsheet library.
2. Run PHPStan on the project.

Proposed resolution

If the Views Data Export module is indeed using the PhpOffice\PhpSpreadsheet library, it should declare it as a dependency in its composer.json file. This way, the library will be automatically installed when the module is installed via Composer. Alternatively, if the library is not actually needed, the code should be updated to remove the references to it.

Remaining tasks

1. Currently, code is inaccessible because the format is never set to xls/xlsx.
Options for next step:
a. Update the module's composer.json file to declare this library as a dependency, if it's needed and make code accessible.
b. Update the module's code to remove the references to the library, and xls(x).

User interface changes

If Option a) was chosen, UI change of added format options would occur.

API changes

No API changes are required to resolve this issue.

Data model changes

No data model changes are required to resolve this issue.

🐛 Bug report
Status

Closed: works as designed

Version

1.3

Component

Code

Created by

🇺🇸United States sandrymend

Live updates comments and jobs are added and updated live.
  • Needs committer feedback

    The issue needs input from the primary project maintainers, because it affects the project’s long-term roadmap, or because the community was not able to come to a consensus and needs a top-down decision. Use sparingly.

Sign in to follow issues

Comments & Activities

  • Issue created by @sandrymend
  • Status changed to Needs review over 1 year ago
  • Comment over 1 year ago →
    🇨🇦Canada nickdickinsonwilde Victoria, BC (T'So-uke lands)

    Investigated this lightly...

    Currently the code that calls PHPSpreadsheet is inaccessible, so won't be a problem. However that is smelly to have in there. I see two main routes forward:

    - make it actually used to export XLS/XLS(X) files
    - *or* clean those references out and link to https://www.drupal.org/project/views_data_export_phpspreadsheet → in the readme as an option.

    An extension to the first option would be to work with the views_data_export_phpspreadsheet maintainers to merge that in to this so that more options are in this primary module.

  • Status changed to Closed: works as designed over 1 year ago
  • Comment over 1 year ago →
    🇨🇦Canada nickdickinsonwilde Victoria, BC (T'So-uke lands)

    Actually, 30s more investigations... looks like if the xls_serialization module is enabled (which requires PHPSpreadsheet), then XLS/XLSX is available and used and otherwise has no harmful effect, so closing this

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024