[Bulk Ops CLI] remove extra newline from bulk mutation variables #6788
+2
−3
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
We detected some changes at Caution DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release. |
jordanverasamy
commented
Jan 22, 2026
| '{"input":{"id":"gid://shopify/Product/1","title":"New Shirt"}}', | ||
| '{"input":{"id":"gid://shopify/Product/2","title":"Cool Pants"}}', | ||
| '{"input":{"id":"gid://shopify/Product/3","title":"Nice Hat"}}', | ||
| '', |
Contributor
Author
There was a problem hiding this comment.
(This empty string right here should've been a smoking gun indicating a bug before, but we all missed it in review!)
Contributor
Coverage report
Show new covered files 🐣
Show files with reduced coverage 🔻
Test suite run success3684 tests passing in 1431 suites. Report generated by 🧪jest coverage report action from 1ad7094 |
ericlee878
approved these changes
Jan 22, 2026
ericlee878
reviewed
Jan 22, 2026
| const {adminSession, variablesJsonl} = options | ||
|
|
||
| const buffer = Buffer.from(variablesJsonl ? `${variablesJsonl}\n` : '', 'utf-8') | ||
| const buffer = Buffer.from(variablesJsonl ?? '', 'utf-8') |
Contributor
There was a problem hiding this comment.
Why did we add an extra ? here?
Contributor
Author
There was a problem hiding this comment.
Good question!! That is the nullish coalescing operator.
Basically, I first removed the \n, and I got this:
variablesJsonl ? variablesJsonl : ''This is a common pattern, and is of course equivalent to:
variablesJsonl || ''?? is an operator that works like ||, except that if the left value is defined-but-falsy (like e.g. ''), || will fall back to the right value, whereas ?? will respect that defined value.
In this case the only falsy value you'll reasonably get here is '', in which case whether we use that or the fallback we'll get an empty string either way. So, ?? and || are going to behave equivalently in all cases here.
I lean towards ?? by default because I don't want to think about JS's weird falsy values unless I have to, so I ended up with:
variablesJsonl ?? ''
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
WHY are these changes introduced?
Fixes https://github.com/shop/issues-api-foundations/issues/1313.
WHAT is this pull request doing?
Thanks to Rebecca's testing we discovered we're just... appending an extra newline, so if your input line already has one, we end up with an extra line? That's not right, so let's not do it.
I think this was being masked before because we were stripping whitespace, but since we've been changing whitespace handling, this is a real bug now.
How to test your changes?
Create
products.jsonlwith a trailing newline:{ "input": { "title": "Sweet new snowboard 1", "productType": "Snowboard", "vendor": "JadedPixel" } } { "input": { "title": "Sweet new snowboard 2", "productType": "Snowboard", "vendor": "JadedPixel" } } { "input": { "title": "Sweet new snowboard 3", "productType": "Snowboard", "vendor": "JadedPixel" } } { "input": { "title": "Sweet new snowboard 4", "productType": "Snowboard", "vendor": "JadedPixel" } } { "input": { "title": "Sweet new snowboard 5", "productType": "Snowboard", "vendor": "JadedPixel" } } { "input": { "title": "Sweet new snowboard 6", "productType": "Snowboard", "vendor": "JadedPixel" } } { "input": { "title": "Sweet new snowboard 7", "productType": "Snowboard", "vendor": "JadedPixel" } } { "input": { "title": "Sweet new snowboard 8", "productType": "Snowboard", "vendor": "JadedPixel" } } { "input": { "title": "Sweet new snowboard 9", "productType": "Snowboard", "vendor": "JadedPixel" } } { "input": { "title": "Sweet new snowboard 10", "productType": "Snowboard", "vendor": "JadedPixel" } }Then run this command:
On main: user error on the last line.
On this branch: no error on the last line!