theoephraim / node-google-spreadsheet

Google Sheets API wrapper for Javascript / Typescript
https://theoephraim.github.io/node-google-spreadsheet
The Unlicense
2.33k stars 390 forks source link

support duplicate headers #673

Open MathRobin opened 9 months ago

MathRobin commented 9 months ago

In this library code, you provid mechanism to forbid duplicate headers for non-empty strings. But this is too coercitive. A google spreadsheet can have duplicate header values Capture d’écran du 2024-01-02 16-32-00 I put as attachment a screenshot which shows that it's possible without any problem.

Maybe you have a business case that need headers to be unique, but it's opinionated.

I tried to only setup data rows without setting header row, but your library doesn't allow it. Anyway to trick this ? Anyway to disable duplicate header control ?

MathRobin commented 9 months ago

Have found a workaround:

worksheet._headerValues = [];
await worksheet.addRows(rows);

quick&dirty but working

vlucas commented 9 months ago

I have this same issue. I find the duplicate header error to be too restrictive, and causes issues with many users in real-world scenarios. Speadsheets from average users are messy, and the code in this library should account for that.

Instead of throwing an error when duplicate headers are found, just pick the first one and use that. So if there are two headers named "Date" or whatever, just use the first one you find and ignore the other. This will ensure that data always gets inserted, even if the actual spreadsheet is a bit messy.

MathRobin commented 9 months ago

Agree and disagree. Ok that spreadsheet of average users are messy but it could be a use-case too, to have duplicate headers (aka "first row") when it's only for calc or storage (for a formula in another sheet).

So I think the best solution is to provide setHeaders method without control of duplicate, or with optional control. We shouldn't use the solution : "take care of first occurence, forget the others", which is opinionated too.

MathRobin commented 9 months ago

I tried to build a PR (for this and for my other bug report) but wasn't able to build project locally just after clone. Any help @theoephraim ?

theoephraim commented 9 months ago

Of course you can have whatever data in whatever cells you want as it is just a spreadsheet. However the row-based API provided by this module is making an explicit trade-off of simplicity in exchange for some limitations and is designed for the use case of when you want to use your spreadsheet like a database. In a database you cannot have duplicate columns. This api also lets us convert rows into objects - using columns as keys - where you cannot have duplicate keys.

In my opinion, in the case of duplicate headers the most reasonable/intuitive thing to do is explode if we detect dupes... Taking the first one would also certainly work, but I think it would cause more confusion than the convenience is worth.

Any other solution would mean adding complexity to how you interact with rows and is in my opinion probably not worth it. Especially since 99% of the time if you are using this module you will also have control over the spreadsheet, it seems fairly reasonable to me to just say - dont use duplicate headers. You can easily adjust one to make it more explicit or even just add a number to avoid the duplicate value.

You can always fall back to the cell-based methods if you want to write any data into any cells and are not using your sheet like a database.

That all said, if you want to propose some way to support duplicate headers that keeps things simple and won't lead to unexpected confusion, I'll certainly consider it... but I can't think of any easy way to do that.

@MathRobin - I'll need more details about the errors you are getting when trying to run/build the project locally... Feel free to open a new issue with that info.

MathRobin commented 9 months ago

@theoephraim i was trying to build with yarn and just realized that you used pnpm on this project. When trying with pnpm, everything is building. So it's ok now to suggest some changes :smile_cat:

abhishekpathak-bitkraft commented 4 months ago

Is the issue fixed? (for confirmation) I am still getting error while createHeader: Error: Duplicate header detected: "apple". Please make sure all non-empty headers are unique