Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support duplicate headers #673

Open
MathRobin opened this issue Jan 2, 2024 · 6 comments
Open

support duplicate headers #673

MathRobin opened this issue Jan 2, 2024 · 6 comments

Comments

@MathRobin
Copy link

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
Copy link
Author

Have found a workaround:

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

quick&dirty but working

@vlucas
Copy link

vlucas commented Jan 2, 2024

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
Copy link
Author

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
Copy link
Author

MathRobin commented Jan 2, 2024

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
Copy link
Owner

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.

@theoephraim theoephraim changed the title Bug - a worksheet can have duplicate headers support duplicate headers Jan 2, 2024
@MathRobin
Copy link
Author

@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 😸

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants