fix: refactor node.ideallyInert to node.inert #8602
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,9 +11,8 @@ const { existsSync } = require('node:fs') | |
| const ssri = require('ssri') | ||
|
|
||
| class Diff { | ||
| constructor ({ actual, ideal, filterSet, shrinkwrapInflated, omit }) { | ||
| this.omit = omit | ||
| this.filterSet = filterSet | ||
| this.shrinkwrapInflated = shrinkwrapInflated | ||
| this.children = [] | ||
|
|
@@ -39,7 +38,6 @@ class Diff { | |
| filterNodes = [], | ||
| shrinkwrapInflated = new Set(), | ||
| omit = new Set(), | ||
| }) { | ||
| // if there's a filterNode, then: | ||
| // - get the path from the root to the filterNode. The root or | ||
|
|
@@ -98,28 +96,18 @@ class Diff { | |
| } | ||
|
|
||
| return depth({ | ||
| tree: new Diff({ actual, ideal, filterSet, shrinkwrapInflated, omit }), | ||
| getChildren, | ||
| leave, | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| const getAction = ({ actual, ideal }) => { | ||
| if (!ideal) { | ||
| return 'REMOVE' | ||
| } | ||
|
|
||
| // bundled meta-deps are copied over to the ideal tree when we visit it, | ||
| // so they'll appear to be missing here. There's no need to handle them | ||
| // in the diff, though, because they'll be replaced at reify time anyway | ||
|
|
@@ -199,7 +187,6 @@ const getChildren = diff => { | |
| filterSet, | ||
| shrinkwrapInflated, | ||
| omit, | ||
| } = diff | ||
|
|
||
| // Note: we DON'T diff fsChildren themselves, because they are either | ||
|
|
@@ -231,7 +218,6 @@ const getChildren = diff => { | |
| filterSet, | ||
| shrinkwrapInflated, | ||
| omit, | ||
| }) | ||
| } | ||
|
|
||
|
|
@@ -251,21 +237,32 @@ const diffNode = ({ | |
| filterSet, | ||
| shrinkwrapInflated, | ||
| omit, | ||
| }) => { | ||
| if (filterSet.size && !(filterSet.has(ideal) || filterSet.has(actual))) { | ||
| return | ||
| } | ||
|
|
||
| if (ideal?.shouldOmit?.(omit)) { | ||
| ideal.inert = true | ||
| } | ||
|
|
||
| // Treat inert nodes as undefined for the purposes of diffing. | ||
| if (ideal?.inert) { | ||
| ideal = undefined | ||
| } | ||
| if (!actual && !ideal) { | ||
| return | ||
| } | ||
|
|
||
| const action = getAction({ actual, ideal }) | ||
|
|
||
| // if it's a match, then get its children | ||
| // otherwise, this is the child diff node | ||
| if (action || (!shrinkwrapInflated.has(ideal) && ideal.hasShrinkwrap)) { | ||
| if (action === 'REMOVE') { | ||
| removed.push(actual) | ||
| } | ||
| children.push(new Diff({ actual, ideal, filterSet, shrinkwrapInflated, omit })) | ||
| } else { | ||
| unchanged.push(ideal) | ||
| // !*! Weird dirty hack warning !*! | ||
|
|
@@ -306,7 +303,6 @@ const diffNode = ({ | |
| filterSet, | ||
| shrinkwrapInflated, | ||
| omit, | ||
| })) | ||
| } | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -101,7 +101,7 @@ class Node { | |
| global = false, | ||
| dummy = false, | ||
| sourceReference = null, | ||
| inert = false, | ||
wraithgar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } = options | ||
| // this object gives querySelectorAll somewhere to stash context about a node | ||
| // while processing a query | ||
|
|
@@ -207,7 +207,7 @@ class Node { | |
| this.extraneous = false | ||
| } | ||
|
|
||
| this.inert = inert | ||
|
|
||
| this.edgesIn = new Set() | ||
| this.edgesOut = new CaseInsensitiveMap() | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We normally do not allow changes to the
package-lock.jsonfile. It makes sense that this change is here now given that this is an optional dep ofchokidar.In order that we don't complicate things I removed the
package-lock.jsonfrom my local copy of this branch and rannpm run resetdepsto verify that what's in here now is valid. The package lock was identical, so we're good to allow it in this PR as changed.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only updated this after CI failed complaining about mismatched package lock. This is the result of fixing missing optional deps here: https://github.com/npm/cli/pull/8602/files#diff-02626074e1a4a170693607e4a3a69dfc08ee52067734717833b22cf162923e07R354