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

[BUG] package-lock.json double-spaced when package.json has a blank line after the opening brace #3

Closed
2 tasks done
MattF-NSIDC opened this issue Sep 30, 2022 · 14 comments
Labels
bug Something isn't working

Comments

@MattF-NSIDC
Copy link

MattF-NSIDC commented Sep 30, 2022

Is there an existing issue for this?

  • I have searched the existing issues

This issue exists in the latest npm version

  • I am using the latest npm

Current Behavior

When running npm install, the package-lock.json output is double-spaced when a blank line exists between the opening brace and the first key of package.json.

This package.json produces a double-spaced lockfile:

{

  "name": "test-doublespace",
  "dependencies": {
    "left-pad": "latest"
  }
}

and this one doesn't:

{
  "name": "test-doublespace",
  "dependencies": {
    "left-pad": "latest"
  }
}

The double-spaced lockfile looks like this all the way to the end:

{

  "name": "test-doublespace",

  "lockfileVersion": 2,

  "requires": true,

  "packages": {
<snip>

I've tried putting blank lines in other places, but have only been able to reproduce it with a blank line after the opening brace.

Expected Behavior

Blank lines are valid in JSON and are not considered to be "data", so I would expect any blank lines to have no impact on the locked data.

Steps To Reproduce

  1. Create a new empty directory
  2. Create a test `package.json
    echo '{
    
      "name": "test-doublespace",
      "dependencies": {
        "left-pad": "latest"
      }
    }' > package.json
    
  3. Run npm install
  4. View package-lock.json. It should be double-spaced!

Environment

  • npm: 8.19.2
  • Node.js: v16.17.1
  • OS Name: Ubuntu 20.04
  • System Model Name: Lemur Pro
  • npm config:
; "user" config from /home/mfisher/.npmrc

//registry.npmjs.org/:_authToken = (protected) 

; node bin location = /home/mfisher/.nvm/versions/node/v16.17.1/bin/node
; node version = v16.17.1
; npm local prefix = /tmp/foo
; npm version = 8.19.2
; cwd = /tmp/foo
; HOME = /home/mfisher
; Run `npm config ls -l` to show all defaults.
@MattF-NSIDC MattF-NSIDC added the bug Something isn't working label Sep 30, 2022
@MattF-NSIDC
Copy link
Author

There is something about the simplicity of reproducing and the weirdness of the behavior that made this special. This added a hard-to-explain amount of fun to my work day. Thank you for the gift of this bug 🙇

@MattF-NSIDC MattF-NSIDC changed the title [BUG] package-lock.json double-spaced when package.json has an empty line after the opening brace [BUG] package-lock.json double-spaced when package.json has a blank line after the opening brace Sep 30, 2022
@MattF-NSIDC MattF-NSIDC changed the title [BUG] package-lock.json double-spaced when package.json has a blank line after the opening brace [BUG] package-lock.json double-spaced when package.json has a blank line after the opening brace Sep 30, 2022
@ljharb
Copy link

ljharb commented Sep 30, 2022

It seems like it's not really a bug - it's picking up the first-used empty-line formatting from the package.json and replicating it in the lockfile.

@MattF-NSIDC
Copy link
Author

Is that intended? Are there other things NPM will do to "transfer" formatting from package.json to package-lock.json? I've tried these package.jsons and they all produce the same lockfile (the example in the OP being the exception). My expectation was that formatting of package.json doesn't matter.

{
  "name": "test-doublespace",
  "dependencies": {
    "left-pad": "latest"
  }
}
{ "name": "test-doublespace",
  "dependencies": {
    "left-pad": "latest"
  }
}
{"name": "test-doublespace", "dependencies": {"left-pad": "latest"}}

@MattF-NSIDC
Copy link
Author

FWIW I just tested with multiple blank lines. Each blank line added to package.json is also added between every line of package-lock.json:

package.json:

{



  "name": "test-doublespace",
  "dependencies": {"left-pad": "latest"}
}

produces package-lock.json:

{



  "name": "test-doublespace",



  "lockfileVersion": 2,



  "requires": true,



  "packages": {



    "": {



      "name": "test-doublespace",



      "dependencies": {



        "left-pad": "latest"



<snip>

@ljharb
Copy link

ljharb commented Sep 30, 2022

Yes, npm copies "is there a trailing newline" and also "indentation" - ie, tabs are preserved, as is the number of spaces you used.

@MattF-NSIDC
Copy link
Author

MattF-NSIDC commented Sep 30, 2022

The indentation behavior is intuitive to me, but replicating the newlines certainly wasn't. Is the replication of newlines a new feature in NPM v8? v7 "transfers" indentation from package.json to package-lock.json, but does not broadcast blank lines like v8 does.

I suppose I don't understand the use case for this behavior!

@ljharb
Copy link

ljharb commented Sep 30, 2022

I don't claim to understand it entirely either, I just assume it's an artifact of the normal formatting preservation behavior.

@lukekarrys
Copy link

I believe the culprit here is our json parse package, specifically these lines.

This behavior looks like an unintended side effect of respecting indentation and newlines, but I think a fix for not double (or triple) spacing these lines would be nice.

@lukekarrys lukekarrys transferred this issue from npm/cli Oct 6, 2022
@lukekarrys
Copy link

I transferred this issue to the json-parse-even-better-errors repo for easier tracking with the eventual fix.

@isaacs
Copy link

isaacs commented May 5, 2023

Yeah, it's trying to figure out what kind of formatting you're trying to use, and stick to that.

It doesn't maintain the context of every token in the json file, making the edit right at the point of change, like a human editing a file would. That's theoretically possible, but not without a much more elaborate setup that would probably be overkill in 99% of cases.

I recommend closing this as wontfix, since it is an edge case that's unlikely to ever be addressed in a satisfying way.

@MattF-NSIDC
Copy link
Author

MattF-NSIDC commented Aug 7, 2023

Weird, I received a notification for this ticket 2 hours ago, but I don't see why 😕 Anyway, it's got me thinking about it again, so will dump some thoughts. Apologies if I re-hash anything that's settled.

Yeah, it's trying to figure out what kind of formatting you're trying to use, and stick to that. [...] like a human editing a file

I think we've established this is an intended feature, but what is the value in attempting to preserve, mimic, or transfer formatting patterns from a human-managed spec to the machine-managed lock of that spec? Does this feature add value, detract from value, or neither?

When I first opened this issue, I was looking at the machine-managed lock file with extra blank lines and thinking "oh jeez, this is way different than I expected from my last projects. I've just stumbled upon a really weird bug. Can I trust this lock file?" From a machine-managed file, I expect predictability, and seeing format surprises inconsistent with my previous experience set off the alarm bells in my head.

I tested some of the other environment management tools I use to see if they do anything like this. Conda creates lockfiles from environments, not from other files, so not a good comparison. conda-lock creates a YAML lock-file from a YAML input file and does not preserve indentation or newlines (no blank lines, always 2-space indent). Poetry creates a TOML lock file from TOML input file and does not preserve indentation or newlines (1 blank line between each section, always 4-space indent). Because these other lockfiles always have consistent formatting, I can't be surprised by formatting.

For package-lock.json, I propose to drop this feature to reduce "astonishment factor" and code maintenance burden. Thanks all for your time thinking about this issue :)

EDIT: Forgot to test yarn. Yarn creates an ASCII lock file from JSON input and does not preserve indentation or newlines (header, then 2 blank lines, then each dependency is separated by 1 blank line. Always 2-space indent).

@isaacs
Copy link

isaacs commented Aug 8, 2023

I think we've established this is an intended feature, but what is the value in attempting to preserve, mimic, or transfer formatting patterns from a human-managed spec to the machine-managed lock of that spec? Does this feature add value, detract from value, or neither?

Historically, some users have expressed a desire to have their package-lock.json use "the same" line break and indentation style as their package.json file, and to have that indentation and line break style preserved over time. This isn't only preference, it can be a matter of accessibility, and line breaks are OS specific, etc. Rather than add yet another config option, we opted to infer the style from the style that is in place in package.json. If that style isn't consistently applied, ok, well, it's limited in what it can infer, and you'll get the first line break and indentation it encounters in a way that is applied consistently.

I would be extremely surprised if this ever changes. Maybe those people complaining about their package-lock not using 4-space or \t indentation or double-spacing just like their package.json file were just a vocal minority, and it would've been better to tell them "too bad". But there are better hills to climb at this point. No one cares enough.

@MattF-NSIDC
Copy link
Author

Thanks for providing the additional context! :) OK to close this? I only thought about it again because of the mystery notification I got from GitHub yesterday.

ertw added a commit to ertw/cli that referenced this issue Apr 16, 2024
Per npm/json-parse-even-better-errors#3 (comment), npm will implicitly update line endings, spacing, etc of `package-lock.json` so that it matches `package.json` 

I'd like to add this documentation because it does not seem to be documented anywhere, and as it is an implicit and non-configurable behavior, it took me a long time to figure out the cause.
wraithgar pushed a commit to npm/cli that referenced this issue Apr 16, 2024
## What

Per
npm/json-parse-even-better-errors#3 (comment),
npm will implicitly update line endings, spacing, etc of
`package-lock.json` so that it matches `package.json`

## Why
I'd like to add this documentation because it does not seem to be
documented anywhere, and as it is an implicit and non-configurable
behavior, it took me a long time to figure out the cause.

## References

npm/json-parse-even-better-errors#3 (comment)
@hashtagchris
Copy link

Thanks for reporting the issue. This behavior, while weird, is an edge case and is cosmetic.

@hashtagchris hashtagchris closed this as not planned Won't fix, can't repro, duplicate, stale Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants
@isaacs @ljharb @lukekarrys @hashtagchris @MattF-NSIDC and others