Skip to content

Commit

Permalink
fix(refactor): symbol cleanup
Browse files Browse the repository at this point in the history 
This also moved the bin to its own directory
  • Loading branch information
@lukekarrys
lukekarrys committed May 6, 2024
1 parent 005d8a9 commit ef45f85
Show file tree
Hide file tree
Showing 11 changed files with 117 additions and 145 deletions.
1 change: 0 additions & 1 deletion lib/bin.js → bin/index.js
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#!/usr/bin/env node
/* eslint-disable no-console */

const run = conf => {
const pacote = require('../')
Expand Down
13 changes: 5 additions & 8 deletions lib/dir.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,10 @@ const { Minipass } = require('minipass')
const tarCreateOptions = require('./util/tar-create-options.js')
const packlist = require('npm-packlist')
const tar = require('tar')
const _prepareDir = Symbol('_prepareDir')
const { resolve } = require('path')
const _readPackageJson = Symbol.for('package.Fetcher._readPackageJson')

const runScript = require('@npmcli/run-script')
const _ = require('./util/protected.js')

const _tarballFromResolved = Symbol.for('pacote.Fetcher._tarballFromResolved')
class DirFetcher extends Fetcher {
constructor (spec, opts) {
super(spec, opts)
Expand All @@ -30,7 +27,7 @@ class DirFetcher extends Fetcher {
return ['directory']
}

[_prepareDir] () {
[_.prepareDir] () {
return this.manifest().then(mani => {
if (!mani.scripts || !mani.scripts.prepare) {
return
Expand All @@ -55,7 +52,7 @@ class DirFetcher extends Fetcher {
})
}

[_tarballFromResolved] () {
[_.tarballFromResolved] () {
if (!this.tree && !this.Arborist) {
throw new Error('DirFetcher requires either a tree or an Arborist constructor to pack')
}
Expand All @@ -68,7 +65,7 @@ class DirFetcher extends Fetcher {

// run the prepare script, get the list of files, and tar it up
// pipe to the stream, and proxy errors the chain.
this[_prepareDir]()
this[_.prepareDir]()
.then(async () => {
if (!this.tree) {
const arb = new this.Arborist({ path: this.resolved })
Expand All @@ -87,7 +84,7 @@ class DirFetcher extends Fetcher {
return Promise.resolve(this.package)
}

return this[_readPackageJson](this.resolved)
return this[_.readPackageJson](this.resolved)
.then(mani => this.package = {
...mani,
_integrity: this.integrity && String(this.integrity),
Expand Down
69 changes: 26 additions & 43 deletions lib/fetcher.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,27 +18,12 @@ const getContents = require('@npmcli/installed-package-contents')
const PackageJson = require('@npmcli/package-json')
const { Minipass } = require('minipass')
const cacheDir = require('./util/cache-dir.js')
const _ = require('./util/protected.js')

// Pacote is only concerned with the package.json contents
const packageJsonPrepare = (p) => PackageJson.prepare(p).then(pkg => pkg.content)
const packageJsonNormalize = (p) => PackageJson.normalize(p).then(pkg => pkg.content)

// Private methods.
// Child classes should not have to override these.
// Users should never call them.
const _extract = Symbol('_extract')
const _mkdir = Symbol('_mkdir')
const _empty = Symbol('_empty')
const _toFile = Symbol('_toFile')
const _tarxOptions = Symbol('_tarxOptions')
const _entryMode = Symbol('_entryMode')
const _istream = Symbol('_istream')
const _assertType = Symbol('_assertType')
const _tarballFromCache = Symbol('_tarballFromCache')
const _tarballFromResolved = Symbol.for('pacote.Fetcher._tarballFromResolved')
const _cacheFetches = Symbol.for('pacote.Fetcher._cacheFetches')
const _readPackageJson = Symbol.for('package.Fetcher._readPackageJson')

class FetcherBase {
constructor (spec, opts) {
if (!opts || typeof opts !== 'object') {
Expand All @@ -57,7 +42,7 @@ class FetcherBase {
this.from = this.spec.registry
? `${this.spec.name}@${this.spec.rawSpec}` : this.spec.saveSpec

this[_assertType]()
this.#assertType()
// clone the opts object so that others aren't upset when we mutate it
// by adding/modifying the integrity value.
this.opts = { ...opts }
Expand Down Expand Up @@ -93,11 +78,9 @@ class FetcherBase {
this.before = opts.before
this.fullMetadata = this.before ? true : !!opts.fullMetadata
this.fullReadJson = !!opts.fullReadJson
if (this.fullReadJson) {
this[_readPackageJson] = packageJsonPrepare
} else {
this[_readPackageJson] = packageJsonNormalize
}
this[_.readPackageJson] = this.fullReadJson
? packageJsonPrepare
: packageJsonNormalize

// rrh is a registry hostname or 'never' or 'always'
// defaults to registry.npmjs.org
Expand Down Expand Up @@ -188,7 +171,7 @@ class FetcherBase {
// private, should be overridden.
// Note that they should *not* calculate or check integrity or cache,
// but *just* return the raw tarball data stream.
[_tarballFromResolved] () {
[_.tarballFromResolved] () {
throw this.notImplementedError
}

Expand All @@ -204,17 +187,17 @@ class FetcherBase {

// private
// Note: cacache will raise a EINTEGRITY error if the integrity doesn't match
[_tarballFromCache] () {
#tarballFromCache () {
return cacache.get.stream.byDigest(this.cache, this.integrity, this.opts)
}

get [_cacheFetches] () {
get [_.cacheFetches] () {
return true
}

[_istream] (stream) {
#istream (stream) {
// if not caching this, just return it
if (!this.opts.cache || !this[_cacheFetches]) {
if (!this.opts.cache || !this[_.cacheFetches]) {
// instead of creating a new integrity stream, we only piggyback on the
// provided stream's events
if (stream.hasIntegrityEmitter) {
Expand Down Expand Up @@ -267,7 +250,7 @@ class FetcherBase {
return false
}

[_assertType] () {
#assertType () {
if (this.types && !this.types.includes(this.spec.type)) {
throw new TypeError(`Wrong spec type (${
this.spec.type
Expand Down Expand Up @@ -306,7 +289,7 @@ class FetcherBase {
!this.preferOnline &&
this.integrity &&
this.resolved
) ? streamHandler(this[_tarballFromCache]()).catch(er => {
) ? streamHandler(this.#tarballFromCache()).catch(er => {
if (this.isDataCorruptionError(er)) {
log.warn('tarball', `cached data for ${
this.spec
Expand All @@ -329,7 +312,7 @@ class FetcherBase {
}. Extracting by manifest.`)
}
return this.resolve().then(() => retry(tryAgain =>
streamHandler(this[_istream](this[_tarballFromResolved]()))
streamHandler(this.#istream(this[_.tarballFromResolved]()))
.catch(streamErr => {
// Most likely data integrity. A cache ENOENT error is unlikely
// here, since we're definitely not reading from the cache, but it
Expand All @@ -352,24 +335,24 @@ class FetcherBase {
return cacache.rm.content(this.cache, this.integrity, this.opts)
}

[_empty] (path) {
#empty (path) {
return getContents({ path, depth: 1 }).then(contents => Promise.all(
contents.map(entry => fs.rm(entry, { recursive: true, force: true }))))
}

async [_mkdir] (dest) {
await this[_empty](dest)
async #mkdir (dest) {
await this.#empty(dest)
return await fs.mkdir(dest, { recursive: true })
}

// extraction is always the same. the only difference is where
// the tarball comes from.
async extract (dest) {
await this[_mkdir](dest)
return this.tarballStream((tarball) => this[_extract](dest, tarball))
await this.#mkdir(dest)
return this.tarballStream((tarball) => this.#extract(dest, tarball))
}

[_toFile] (dest) {
#toFile (dest) {
return this.tarballStream(str => new Promise((res, rej) => {
const writer = new fsm.WriteStream(dest)
str.on('error', er => writer.emit('error', er))
Expand All @@ -383,15 +366,15 @@ class FetcherBase {
}))
}

// don't use this[_mkdir] because we don't want to rimraf anything
// don't use this.#mkdir because we don't want to rimraf anything
async tarballFile (dest) {
const dir = dirname(dest)
await fs.mkdir(dir, { recursive: true })
return this[_toFile](dest)
return this.#toFile(dest)
}

[_extract] (dest, tarball) {
const extractor = tar.x(this[_tarxOptions]({ cwd: dest }))
#extract (dest, tarball) {
const extractor = tar.x(this.#tarxOptions({ cwd: dest }))
const p = new Promise((resolve, reject) => {
extractor.on('end', () => {
resolve({
Expand All @@ -416,7 +399,7 @@ class FetcherBase {

// always ensure that entries are at least as permissive as our configured
// dmode/fmode, but never more permissive than the umask allows.
[_entryMode] (path, mode, type) {
#entryMode (path, mode, type) {
const m = /Directory|GNUDumpDir/.test(type) ? this.dmode
: /File$/.test(type) ? this.fmode
: /* istanbul ignore next - should never happen in a pkg */ 0
Expand All @@ -427,7 +410,7 @@ class FetcherBase {
return ((mode | m) & ~this.umask) | exe | 0o600
}

[_tarxOptions] ({ cwd }) {
#tarxOptions ({ cwd }) {
const sawIgnores = new Set()
return {
cwd,
Expand All @@ -437,7 +420,7 @@ class FetcherBase {
if (/Link$/.test(entry.type)) {
return false
}
entry.mode = this[_entryMode](entry.path, entry.mode, entry.type)
entry.mode = this.#entryMode(entry.path, entry.mode, entry.type)
// this replicates the npm pack behavior where .gitignore files
// are treated like .npmignore files, but only if a .npmignore
// file is not present.
Expand Down
15 changes: 6 additions & 9 deletions lib/file.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,7 @@ const cacache = require('cacache')
const { resolve } = require('path')
const { stat, chmod } = require('fs/promises')
const Fetcher = require('./fetcher.js')

const _exeBins = Symbol('_exeBins')
const _tarballFromResolved = Symbol.for('pacote.Fetcher._tarballFromResolved')
const _readPackageJson = Symbol.for('package.Fetcher._readPackageJson')
const _ = require('./util/protected.js')

class FileFetcher extends Fetcher {
constructor (spec, opts) {
Expand All @@ -27,7 +24,7 @@ class FileFetcher extends Fetcher {
// have to unpack the tarball for this.
return cacache.tmp.withTmp(this.cache, this.opts, dir =>
this.extract(dir)
.then(() => this[_readPackageJson](dir))
.then(() => this[_.readPackageJson](dir))
.then(mani => this.package = {
...mani,
_integrity: this.integrity && String(this.integrity),
Expand All @@ -36,7 +33,7 @@ class FileFetcher extends Fetcher {
}))
}

[_exeBins] (pkg, dest) {
#exeBins (pkg, dest) {
if (!pkg.bin) {
return Promise.resolve()
}
Expand Down Expand Up @@ -65,11 +62,11 @@ class FileFetcher extends Fetcher {
// but if not, read the unpacked manifest and chmod properly.
return super.extract(dest)
.then(result => this.package ? result
: this[_readPackageJson](dest).then(pkg =>
this[_exeBins](pkg, dest)).then(() => result))
: this[_.readPackageJson](dest).then(pkg =>
this.#exeBins(pkg, dest)).then(() => result))
}

[_tarballFromResolved] () {
[_.tarballFromResolved] () {
// create a read stream and return it
return new fsm.ReadStream(this.resolved)
}
Expand Down
Loading

0 comments on commit ef45f85

Please sign in to comment.