Skip to content

Commit

Permalink
fix: the package tree of bundled dependencies was incorrect
Browse files Browse the repository at this point in the history
  • Loading branch information
Benjamin Coe committed Jun 14, 2016
1 parent b9a3023 commit 7bdccf5
Showing 0 changed files with 0 additions and 0 deletions.

4 comments on commit 7bdccf5

@gr2m
Copy link
Contributor

@gr2m gr2m commented on 7bdccf5 Jun 14, 2016

Choose a reason for hiding this comment

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

@bcoe can you explain what exactly was wrong and how you fixed it? The commit shows no changes, but Greenkeeper reported a ton of fails for nyc@6.6.0. I tried clearing cache and restarting the build to make it install 6.6.1 and that fixed it: https://travis-ci.org/hoodiehq/hoodie/builds/137583994

@bcoe
Copy link
Member

@bcoe bcoe commented on 7bdccf5 Jun 14, 2016

Choose a reason for hiding this comment

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

@gr2m npm's dedupe logic in 3.x brings to the forefront some potential singleton related bugs, because nyc acts as a harness, and relies on several fairly popular dependencies, it runs into a few of these ... as an example require-main-filename reporting the working directory of where nyc included the module first, rather than the top-level dependency that required the module.

tldr; we started bundling dependencies with nyc mainly to ensure that we were hanging these singletons onto different module instances ... we next found that deduping the modules installed for nyc itself still could lead to some of this strange singleton behavior ... my solution was to start installing with npm@2.x prior to publishing nyc ... unflattened dependencies.

v6.6.0 somehow ended up in a state where some deep dependencies had been hoisted that should not have been -- which is a dependency of foreground-child and had been hoisted, but is not part of the list of bundled modules.

@bcoe
Copy link
Member

@bcoe bcoe commented on 7bdccf5 Jun 15, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

clarifying this summary a bit for the benefit of @kt3k:

  1. if a module being tested by nyc, and nyc itself share a module, e.g., yargs, issues can arise when these modules hang state onto the global module.
  2. by using bundled dependencies, we ensure that there are two copies of the module in the node_modules folder.
  3. node's module loader attaches this global state onto each instance of the module, so if we have two different copies of yargs in node_modules, we circumvent singleton related bugs.
  4. I somehow ended up with an unclean publish of nyc, which had deduped some modules but not others.

@kt3k
Copy link

@kt3k kt3k commented on 7bdccf5 Jun 15, 2016

Choose a reason for hiding this comment

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

@bcoe
Now I understand what singleton means in this context!
I think I understand this much clearer! 😄

Thank you! 👍

Please sign in to comment.