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

fix: upgrade foreground-child dependency #276

Merged
merged 1 commit into from
Jun 14, 2016
Merged

Conversation

bcoe
Copy link
Member

@bcoe bcoe commented Jun 13, 2016

@addaleax @jamestalmage mind giving this a spin while builds run?

@jamestalmage jamestalmage mentioned this pull request Jun 13, 2016
@jamestalmage
Copy link
Member

jamestalmage commented Jun 13, 2016

spawn-wrap still depends on [email protected]

@addaleax
Copy link
Member

Looks good locally and I think bundling an npm3-style tree should take care of the old version that spawn-wrap depends on, right?

@bcoe
Copy link
Member Author

bcoe commented Jun 13, 2016

@jamestalmage @addaleax my plan is to bundle on a [email protected] tree, which solves some singleton issues with our dependencies -- but! I think we should be fine as long as I re-install everything prior to publish? spawn-wrap isn't itself bundled or pinned.

@addaleax
Copy link
Member

I guess that should work. If the copy of foreground-child in spawn-wrap’s dependencies is still outdated after that for some reason, you can probably also just remove it and let spawn-wrap use the top-level one.

@bcoe bcoe merged commit 1b9e4a8 into master Jun 14, 2016
@bcoe bcoe deleted the foreground-child-upgrade branch June 14, 2016 01:03
@jamestalmage
Copy link
Member

Yeah - actually, it's a carrot dependency, so it should be deduped.

Why does bundling on npm@2 solve problems?

@bcoe
Copy link
Member Author

bcoe commented Jun 14, 2016

@jamestalmage nyc relies on a few popular dependencies that themselves use a singleton pattern (for various reasons, maintaining state, etc).

Bundling dependencies helped to solve this issue, but I found that deduping dependencies and publishing with [email protected] continued to introduce some regressions:

#246

@jamestalmage
Copy link
Member

Hmm.

Have we figured out why that is happening? Is there a bug somewhere in npm? Ideally we wouldn't be so brittle.

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

Successfully merging this pull request may close these issues.

3 participants