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

stringify: fix arrayFormat comma with empty array/objects #350

merged 1 commit into from Aug 29, 2020


Copy link

Fixed #310

continued from PR #317

Copy link

ljharb commented Jan 15, 2020

Why did you open a second PR? That creates a permanent pollution in the repo refs that can never be removed.

Please never ever make a duplicate PR.

.gitignore Outdated Show resolved Hide resolved
dist/qs.js Outdated Show resolved Hide resolved
Copy link

ljharb commented May 2, 2020

@daggerjames hey, sorry for the delay here :-)

I've rebased this; the first commit is awesome and good to go; for the second one, 3 of your new tests are failing - and it seems like the tests are correct. Can you take a look?

@ljharb ljharb changed the title #310 stringify: fix arrayFormat comma with empty array/objects May 2, 2020
Copy link
Contributor Author

fixed all tests. Seems logical error with previous approach.
could you please rebase the branch for me? thanks. I'm still unfamiliar with those rebase things and might lose it like what I did last time


Copy link

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

Not stoked on the "use an object as a sentinel key" but it seems straightforward enough, and tests pass.

Thanks, and sorry for the repeated delays :-)

@ljharb ljharb merged commit deada94 into ljharb:master Aug 29, 2020
Copy link

Hi @ljharb, sorry to bother you, but were these changes published to npm? I can still reproduce the original behavior from the latest published version.

See this failed test (extracted from this pull request)

Copy link

ljharb commented Nov 18, 2020

No; this hasn’t been published yet.

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

Successfully merging this pull request may close these issues.

arrayformat as comma with empty array or objects
3 participants