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

Allow excluding specific file patterns in nested directories #31

Closed
Shingyx opened this issue May 28, 2019 · 3 comments
Closed

Allow excluding specific file patterns in nested directories #31

Shingyx opened this issue May 28, 2019 · 3 comments

Comments

@Shingyx
Copy link

Shingyx commented May 28, 2019

For a file pattern such as dist/**/!(*.test.js), the package currently adds an entry of !dist/**/!(*.test.js)/** for the ignore-walker, which results in paths such as dist/lib/x.test.js getting included in the pack. I raised the PR #29 as a possible solution, but I've since closed it because I felt it wasn't the right approach.

The following unit test file can be added to assert this behaviour.

'use strict'

const fs = require('fs')
const path = require('path')

const mkdirp = require('mkdirp')
const rimraf = require('rimraf')
const t = require('tap')

const pack = require('../')

const pkg = path.join(__dirname, path.basename(__filename, '.js'))
t.teardown(_ => rimraf.sync(pkg))

const json = {
  name: 'test-package',
  version: '1.6.2',
  files: [
    'dist/**/!(*.test.*)'
  ]
}

const expect = [
  'package.json',
  'dist/index.d.ts',
  'dist/index.js',
  'dist/lib/util.d.ts',
  'dist/lib/util.js'
]

t.test('setup', t => {
  rimraf.sync(pkg)
  mkdirp.sync(pkg)
  fs.writeFileSync(path.join(pkg, 'package.json'), JSON.stringify(json, null, 2))

  const srcDir = path.join(pkg, 'src')
  mkdirp.sync(srcDir)
  fs.writeFileSync(path.join(srcDir, 'index.ts'), 'export const x = () => console.log("index")')
  fs.writeFileSync(path.join(srcDir, 'index.test.ts'), 'import { x } from "./index"')

  const srcLibDir = path.join(srcDir, 'lib')
  mkdirp.sync(srcLibDir)
  fs.writeFileSync(path.join(srcLibDir, 'util.ts'), 'export const y = () => console.log("util")')
  fs.writeFileSync(path.join(srcLibDir, 'util.test.ts'), 'import { y } from "./util"')

  const distDir = path.join(pkg, 'dist')
  mkdirp.sync(distDir)
  fs.writeFileSync(path.join(distDir, 'index.d.ts'), 'export declare function x(): void')
  fs.writeFileSync(path.join(distDir, 'index.js'), 'exports.x = function () { console.log("index") }')
  fs.writeFileSync(path.join(distDir, 'index.test.d.ts'), '')
  fs.writeFileSync(path.join(distDir, 'index.test.js'), 'var x = require("./index").x')

  const distLibDir = path.join(distDir, 'lib')
  mkdirp.sync(distLibDir)
  fs.writeFileSync(path.join(distLibDir, 'util.d.ts'), 'export declare function y(): void')
  fs.writeFileSync(path.join(distLibDir, 'util.js'), 'exports.y = function () { console.log("util") }')
  fs.writeFileSync(path.join(distLibDir, 'util.test.d.ts'), '')
  fs.writeFileSync(path.join(distLibDir, 'util.test.js'), 'var y = require("./util").y')

  t.end()
})

t.test('follows npm package ignoring rules', t => {
  const check = (files, t) => {
    t.same(files, expect)
    t.end()
  }

  t.test('sync', t => check(pack.sync({ path: pkg }), t))
  t.test('async', t => pack({ path: pkg }).then(files => check(files, t)))

  t.end()
})

Thanks!

@isaacs
Copy link
Contributor

isaacs commented Jul 3, 2019

This is a limitation of globs. dist/lib/x.test.js actually does match dist/**/!(*.test.js)/**. First part "dist" matches the first part of the glob. Then the second part ** matches zero folders. The third part !(*.test.js) matches lib. The last part ** matches x.test.js.

I think it should work if you do this, though:

{
  "files": [
    "/dist/",
    "!/dist/**/*.test.js"
  ]
}

The second rule will override the first, and re-ignore those files.

@Shingyx
Copy link
Author

Shingyx commented Jul 4, 2019

That works for my use case. Thanks!

Could you please clarify why the pattern !dist/**/!(*.test.js)/** is passed to the ignore-walker though? Is it necessary? What I specify in the package.json is dist/**/!(*.test.js), i.e. without the /** at the end.

I'm referring to this block in particular - it takes the pattern p and then converts that to !p and !p/** for the ignore-walker.

npm-packlist/index.js

Lines 186 to 188 in 270f534

super.onReadIgnoreFile('package.json', '*\n' + pkg.files.map(
f => '!' + f + '\n!' + f.replace(/\/+$/, '') + '/**'
).join('\n') + '\n', then)

@isaacs
Copy link
Contributor

isaacs commented Oct 9, 2019

Because at that point it does not know if it's a directory or a file.

This behavior is overly aggressive in what it includes, though, and leads to problems. For example, "files": ["lib"] will include lib/.DS_Store, which you never want.

That'll be fixed in v2.

@isaacs isaacs closed this as completed Oct 9, 2019
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

No branches or pull requests

2 participants