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

feat!: major refactor #88

Merged
merged 1 commit into from
Sep 21, 2022
Merged

feat!: major refactor #88

merged 1 commit into from
Sep 21, 2022

Conversation

nlf
Copy link
Contributor

@nlf nlf commented Apr 6, 2022

BREAKING CHANGE: this module now follows a strict order of operations when applying ignore rules. if a package.json containing a 'files' array is present, then top level .npmignore and .gitignore files will both be ignored entirely

this refactor drops the previous approach where the package.json files array was used to direct the tree walker, rather than as an ignore list.

the files array is back to being an ignore list, and may be overridden by nested .npmignore and .gitignore files (i.e. not at the project root).

in addition, rather than using npm-bundled to gather a list of flat package names that are to be bundled in this module, we instead load an arborist tree at the project root and walk the actual tree structure to determine what packages (and from where) should be bundled. this allowed us to reduce the complexity of the code here quite a bit since we're doing several small tree walks that do not traverse node_modules at all, so we don't have to keep track of which directories should be ignored. we also now force the follow option passed to ignore-walk to false as we no longer have any need to traverse a symlinked directory at all.

bundled dependencies are now handled somewhat differently as well. if a bundled dependency is a symlink, we assume that it is either a workspace or an otherwise linked standalone project and treat it as such. if a bundled dependency is not a symlink, then we skip all ignore rules except for those found in the package.json of the bundled dep as well as any strict inclusions or exclusions we determine based on other fields in the package.json.

closes #50
closes #54
closes #66

@nlf nlf requested a review from a team as a code owner April 6, 2022 20:28
@nlf nlf force-pushed the nlf/refactor-phase-three branch 4 times, most recently from f2876a2 to 4443c4a Compare April 6, 2022 21:17
Copy link
Contributor

@fritzy fritzy left a comment

Choose a reason for hiding this comment

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

Quite the rewrite!

lib/index.js Outdated Show resolved Hide resolved
lib/index.js Show resolved Hide resolved
lib/index.js Show resolved Hide resolved
@nlf nlf force-pushed the nlf/refactor-phase-three branch 3 times, most recently from 08ffbba to ee7189c Compare April 7, 2022 18:35
@darcyclarke darcyclarke mentioned this pull request Aug 22, 2022
32 tasks
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
lib/index.js Show resolved Hide resolved
lib/index.js Outdated Show resolved Hide resolved
lib/index.js Outdated Show resolved Hide resolved
lib/index.js Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
…e more consistent

BREAKING CHANGE: this module now follows a strict order of operations when applying ignore rules. if a `files` array is present in the package.json, then rules in `.gitignore` and `.npmignore` files from the root will be ignored.
@nlf nlf merged commit c37371b into main Sep 21, 2022
@nlf nlf deleted the nlf/refactor-phase-three branch September 21, 2022 20:28
@github-actions github-actions bot mentioned this pull request Sep 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants