Skip to content

feat: publish netlify package #7293

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

Merged
merged 21 commits into from
May 19, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion .github/workflows/release-please.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,18 @@ jobs:
- name: Run E2E tests
run: npm run test:e2e

- name: Publish package
- name: Publish netlify-cli package
run: npm publish --provenance
env:
NODE_AUTH_TOKEN: ${{secrets.NPM_TOKEN}}

- name: Prepare netlify package
run: node scripts/netlifyPackage.js prepare

- name: Publish netlify package
run: npm publish --provenance
env:
NODE_AUTH_TOKEN: ${{secrets.NPM_TOKEN}}

- name: Verify netlify package
run: node scripts/netlifyPackage.js verify
54 changes: 54 additions & 0 deletions scripts/netlifyPackage.js
Copy link
Member

Choose a reason for hiding this comment

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

nit: I was going to comment that this codebase does not follow this naming convention, but I see that scripts/prepublishOnly is the one exception, which you probably saw right next to this one!

🤷🏼

Copy link
Contributor

Choose a reason for hiding this comment

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

I did name scripts/prepublishOnly as it is (despite not matching other file naming conventions) specifically so it would match up exactly with the npm script name.

Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// @ts-check
import assert from 'node:assert'
import { basename, dirname, resolve } from 'node:path'
import { argv } from 'node:process'
import { fileURLToPath } from 'node:url'
import { readFile, writeFile } from 'node:fs/promises'

import execa from 'execa'

const packageJSON = await getPackageJSON()

async function getPackageJSON() {
const packageJSONPath = resolve(fileURLToPath(import.meta.url), '../../package.json')
const contents = JSON.parse(await readFile(packageJSONPath, 'utf8'))
Copy link
Member

Choose a reason for hiding this comment

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

nit: We already have a dep on normalize-package-data you can use here to avoid contents being any

There's even a util src/utils/get-cli-package-json.ts you should be able to use

Copy link
Member Author

Choose a reason for hiding this comment

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

This use case is a bit different. We're reading package.json and then writing it back, so I don't think we should be normalising it.

I have used the Package type now.


return {
contents,
path: packageJSONPath,
}
}

/**
* @type {Record<string, function>}
*/
const commands = {
prepare: async () => {
const newPackageJSON = {
...packageJSON.contents,
main: './dist/index.js',
name: 'netlify',
Copy link
Member Author

Choose a reason for hiding this comment

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

Now I'm wondering if we should also change the name of the binary so that npx netlify and any globally-installed netlify-cli do not collide?

Copy link
Member

Choose a reason for hiding this comment

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

As far as I understand the wonky npm heuristics here:

  • if a user has both netlify and netlify-cli installed (both locally or both globally), npx netlify will run netlify
  • if a user has netlify-cli and does not have netlify, npx netlify will always run netlify-cli rather than try to install netlify
  • if a user has neither installed, npx netlify will install and run netlify

(It's likely worth verifying these)

Does this seem ok or are any of these scenarios problematic?

Copy link
Member Author

Choose a reason for hiding this comment

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

My question is whether npx netlify will also install the binary into the PATH, in the same way that globally-installing a package does. If that's the case, then the netlify binary will overwrite the binary with the same name that had been injected by a globally-installed netlify-cli, which is not intended.

But if npx netlify simply runs the binary without adding it to the PATH (which I feel is the expected behaviour), this should be fine.

}

console.log(`Writing updated package.json to ${packageJSON.path}...`)
await writeFile(packageJSON.path, `${JSON.stringify(newPackageJSON, null, 2)}\n`)

console.log('Re-installing dependencies to update lockfile...')
await execa('npm', ['install'], {
cwd: dirname(packageJSON.path),
})
Copy link
Member

Choose a reason for hiding this comment

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

Hrm, his is getting difficult to reason about... So, before this script runs in CI, we'll have run this as part of the netlify-ci publish step: https://github.com/netlify/cli/blob/47242ce76c7aa57978e14e9e075ca6f0f76af348/scripts/prepublishOnly.js.

This means we'll have mutated package.json to prune dev deps, converted the package-lock.json to an npm-shrinkwrap.json, then restored the original package.json... now here we'll mutate package.json again to update name and add main, then run npm i again, which will restore the dev deps into the npm-shrinkwrap.json, making netlify twice as big to install as netlify-cli. 😰 Did I get all that right?

@ndhoule and I just recently decomplicated all of this (with many false starts due to how tangled and brittle it was), so I think it would be worth avoiding getting back into this state if we can help it. I don't have concrete suggestions that fit in a comment box, but I'm happy to pair on it if you'd like.

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, I hadn't realised that this flow had changed. My bad.

In 70dacdb I have made the following changes:

  1. Changed prepublishOnly.js to stop trying to restore the original state of package.json. This flow is meant to run in the CI only, where trying to restore things has no benefit and will only slow things down.
  2. Changed netlifyPackage.js to run npm shrinkwrap (instead of npm install) so that the shrinkwrap file is updated with the right package name. It will have the same dependencies, as we've preserved the package.json file modified in step 1.

Let me know if that makes sense.

},

verify: async () => {
const { stdout } = await execa('npx', ['-y', 'netlify', '--version'])
const version = stdout.match(/netlify-cli\/(\d+\.\d+\.\d+)/)

assert.equal(version, packageJSON.contents.version, 'Version installed via npx matches latest version')
},
}

if (typeof commands[argv[2]] === 'function') {
await commands[argv[2]]()
} else {
console.error(`Usage: node ${basename(argv[1])} <command> (available commands: ${Object.keys(commands).join(', ')})`)
}
4 changes: 4 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// This is an entrypoint that mirrors the interface that the `netlify` package
Copy link
Member

Choose a reason for hiding this comment

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

thought: Should we log a warning here that the package has been renamed?

Copy link
Member Author

Choose a reason for hiding this comment

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

That feels a bit extreme? We might be logging something that is not actionable to the user, since this might be a transitive dependency.

// used to have, before it was renamed to `@netlify/api`. We keep it for
// backwards-compatibility.
export { NetlifyAPI, methods } from '@netlify/api'
Loading