Skip to content

add package-lock.json#26

Merged
simon-id merged 5 commits into
mainfrom
add-package-lock.json
May 22, 2026
Merged

add package-lock.json#26
simon-id merged 5 commits into
mainfrom
add-package-lock.json

Conversation

@simon-id
Copy link
Copy Markdown
Member

No description provided.

@simon-id simon-id marked this pull request as ready for review April 13, 2026 12:57
Copy link
Copy Markdown
Member

@BridgeAR BridgeAR left a comment

Choose a reason for hiding this comment

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

We need to prevent the package-lock from being in a release. We just want the lockfile working for us. I think it would be good to use files in the package.json instead of using the npmignore file.
In addition we should activate dependabot or similar to update these in an automated way. That should be done at the same time as introducing this in my opinion.

with:
node-version: ${{ matrix.node-version }}
- run: npm install
- run: npm install -g npm@7.24.2 # version of npm compatible with all tested node versions
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would this be updated with dependabot?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

hopefully not ? i had to find a npm version that both works with old and new node versions

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see. That seems like a good reason to create a new major version where we drop support for Node.js versions < 18

@simon-id
Copy link
Copy Markdown
Member Author

simon-id commented Apr 14, 2026

@BridgeAR

We need to prevent the package-lock from being in a release

package-lock.json always gets ignored by npm pack/publish (proof below)
Another thing, this project only has dev deps.

$ npm pack
npm notice
npm notice 📦  dc-polyfill@0.1.10
npm notice Tarball Contents
npm notice 82B LICENSE-3rdparty.csv
npm notice 1.1kB LICENSE.txt
npm notice 134B NOTICE
npm notice 2.8kB README.md
npm notice 1.1kB acquire-channel-registry.js
npm notice 2.2kB checks.js
npm notice 918B dc-polyfill.js
npm notice 440B dc-polyfill.mjs
npm notice 1.4kB errors.js
npm notice 1.0kB package.json
npm notice 1.9kB patch-channel-store-methods.js
npm notice 807B patch-channel-unsubscribe-return.js
npm notice 1.2kB patch-garbage-collection-bug.js
npm notice 530B patch-sync-unsubscribe-bug.js
npm notice 268B patch-top-subscribe-unsubscribe.js
npm notice 630B patch-tracing-channel-has-subscribers.js
npm notice 5.0kB patch-tracing-channel.js
npm notice 1.4kB primordials.js
npm notice 2.6kB reimplementation.js
npm notice Tarball Details
npm notice name: dc-polyfill
npm notice version: 0.1.10
npm notice filename: dc-polyfill-0.1.10.tgz
npm notice package size: 7.9 kB
npm notice unpacked size: 25.5 kB
npm notice shasum: 438c554361c4a67426a390b882be024713a22e4f
npm notice integrity: sha512-C6f/UwEFdO/2m[...]9dm7DIEW/GUAQ==
npm notice total files: 19
npm notice
dc-polyfill-0.1.10.tgz

@simon-id
Copy link
Copy Markdown
Member Author

@BridgeAR I believe your required changes are irrelevant. As I said, the lock file is always ignored by npm publish (proof above), and there isn't much need for dependabot since this project only has dev dependencies.

Copy link
Copy Markdown
Member

@BridgeAR BridgeAR left a comment

Choose a reason for hiding this comment

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

I still think doing the two things I mentioned are better while this improves the situation as is, so I am fine to land it

@simon-id simon-id merged commit 8abaace into main May 22, 2026
43 checks passed
@simon-id simon-id deleted the add-package-lock.json branch May 22, 2026 14:06
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.

2 participants