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

build(deps): Upgrade Papaparse to 5.0.0 #1330

Merged
merged 3 commits into from
Feb 24, 2021

Conversation

ConradJChan
Copy link
Contributor

Tested against sanity folder in all browsers

@ConradJChan ConradJChan requested a review from a team as a code owner February 22, 2021 22:00
mxiao6
mxiao6 previously approved these changes Feb 22, 2021
this.startLoadTimer();
const urlWithAuth = this.createContentUrlWithAuthParams(template);
Papa.parse(urlWithAuth, {
delimitersToGuess: [',', '\t'],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added the option delimitersToGuess which will restrict the delimiters to comma and tab which technically was all that we supported given that we support the extensions CSV and TSV. I could remove this option entirely, but then there is an issue with the auto-detection of delimiter for XSS.csv with the excessive ; as well as ,.

At the end of the day it feels like keeping the delimiters to what we support makes sense. Allowing users to hack the system by naming it csv but providing delimiters such as | or ; doesn't make all that much sense, imo.

Thoughts?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Gotcha, so it sounds like we may need some new options to maintain consistency with the current experience. Should we update the value to [',', '\t', Papa.RECORD_SEP, Papa.UNIT_SEP] to retain support for the "true" delimiters mentioned in the docs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed version to 5.0.0 which doesn't contain the tweaks to the method which aims to guess the delimiter. Will follow up with Papaparse with the issue we're seeing

@ConradJChan ConradJChan changed the title build(deps): Upgrade Papaparse to 5.2.0 build(deps): Upgrade Papaparse to 5.0.0 Feb 24, 2021
@ConradJChan ConradJChan merged commit 9e66b55 into box:master Feb 24, 2021
@ConradJChan ConradJChan deleted the upgrade-papaparse-5.2.0 branch February 24, 2021 23:52
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.

3 participants