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

Exclude pkg main files from `no-unused-modules` #1404

Open
wants to merge 7 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@rfermann
Copy link
Contributor

commented Jul 2, 2019

Implements a solution for #1327

@coveralls

This comment has been minimized.

Copy link

commented Jul 2, 2019

Coverage Status

Coverage increased (+2.6%) to 97.785% when pulling f6d83b2 on rfermann:master into bb9ba24 on benmosher:master.

Show resolved Hide resolved docs/rules/no-unused-modules.md Outdated
Show resolved Hide resolved src/rules/no-unused-modules.js

rfermann and others added some commits Jul 2, 2019

Update docs/rules/no-unused-modules.md
Co-Authored-By: Jordan Harband <ljharb@gmail.com>
return false
}

if (pkg.bin && typeof pkg.bin === 'string') {

This comment has been minimized.

Copy link
@ljharb

ljharb Jul 2, 2019

Collaborator

seems like we can consolidate the logic for “check string field X” and “check object field Y”, so t doesn’t have to be repeated for main/bin/browser and bin/browser?

This comment has been minimized.

Copy link
@rfermann

rfermann Jul 3, 2019

Author Contributor

Good hint. I made the logic a bit more generic with 3ed222d.

return true
}
}

if (pkg.main) {
if (join(basePath, pkg.main) === file) {
if (checkPkgField(pkg.main)) {

This comment has been minimized.

Copy link
@ljharb

ljharb Jul 3, 2019

Collaborator

the only remaining problem here is that main can’t be an object :-/

This comment has been minimized.

Copy link
@rfermann

rfermann Jul 3, 2019

Author Contributor

Maybe I don't see the issue here. main will either return true within the first if branch or just return at the end of the function.
Would it be better to return false in the first if branch to make main not even pass the if branch for object handling?

This comment has been minimized.

Copy link
@ljharb

ljharb Jul 8, 2019

Collaborator

Yes, that's what I'm suggesting. an object "main" field is just invalid and shouldn't be considered.

@ljharb
Copy link
Collaborator

left a comment

Thanks, looks great! Just a couple more changes.

Show resolved Hide resolved docs/rules/no-unused-modules.md Outdated
Show resolved Hide resolved tests/files/no-unused-modules/main/index.js Outdated

rfermann and others added some commits Jul 15, 2019

Update docs/rules/no-unused-modules.md
Co-Authored-By: Jordan Harband <ljharb@gmail.com>
@ljharb

ljharb approved these changes Jul 16, 2019

}

const checkPkgFieldObject = pkgField => {
const pkgFieldFiles = Object.values(pkgField).map(value => join(basePath, value))

This comment has been minimized.

Copy link
@ljharb

ljharb Jul 16, 2019

Collaborator

Object.values is not available on all supported nodes; please use https://npmjs.com/object.values instead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.