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

[fixed] 2.1.0 exports broken in 2.2.0 #15

Closed
ajwhite opened this Issue Mar 28, 2016 · 18 comments

Comments

Projects
None yet
7 participants
@ajwhite

ajwhite commented Mar 28, 2016

Via comments on 7450295#commitcomment-16867578

import ansi from 'ansi-styles';

console.log(`${ansi.colors.red.open}hello world${ansi.colors.red.close}`)

No longer works, with the result of

undefined is not an object (evaluating _ansiStyles2.default.colors.red)


Steps to reproduce:

  1. Install 2.1.0 npm install ansi-styles@2.1.0
  2. Use the code above
  3. Update to 2.2.0 npm install ansi-styles@2.2.0
  4. Re-run the same code, experience error
@Qix-

This comment has been minimized.

Show comment
Hide comment
@Qix-

Qix- Mar 28, 2016

Member

Yeah this should have been a major release....

Either use 2.1.0 (specific semver version) or wait until Sindre can deprecate this and re-publish as a major version bump.

// @jbnicolai

Member

Qix- commented Mar 28, 2016

Yeah this should have been a major release....

Either use 2.1.0 (specific semver version) or wait until Sindre can deprecate this and re-publish as a major version bump.

// @jbnicolai

@Qix- Qix- added bug and removed bug labels Mar 28, 2016

@Qix-

This comment has been minimized.

Show comment
Hide comment
@Qix-

Qix- Mar 28, 2016

Member

Version 2.2.0 has been unpublished. We'll do a major bump sometime tomorrow.

Thanks for bringing this to our attention @ajwhite 👍

Member

Qix- commented Mar 28, 2016

Version 2.2.0 has been unpublished. We'll do a major bump sometime tomorrow.

Thanks for bringing this to our attention @ajwhite 👍

@Qix- Qix- closed this Mar 28, 2016

@ajwhite

This comment has been minimized.

Show comment
Hide comment
@ajwhite

ajwhite Mar 28, 2016

Thanks for the quick recovery!

ajwhite commented Mar 28, 2016

Thanks for the quick recovery!

@Qix-

This comment has been minimized.

Show comment
Hide comment
@Qix-
Member

Qix- commented Mar 28, 2016

ninja-fail_o_588896

@hulbert

This comment has been minimized.

Show comment
Hide comment
@hulbert

hulbert Mar 28, 2016

So you are aware, unpublishing breaks builds that have used shrinkwrap to pin to 2.2.0.

hulbert commented Mar 28, 2016

So you are aware, unpublishing breaks builds that have used shrinkwrap to pin to 2.2.0.

@Qix-

This comment has been minimized.

Show comment
Hide comment
@Qix-

Qix- Mar 28, 2016

Member

@hulbert unpublishing breaks a lot of things, though it was necessary in this case I believe. Our next release will be 3.0.0 anyway.

Would you or @sindresorhus be willing to write some steps on how to fix the shrinkwrap issue if others are facing it? I've not used shrinkwrap personally.


Sorry if this inconvenienced anyone.

Member

Qix- commented Mar 28, 2016

@hulbert unpublishing breaks a lot of things, though it was necessary in this case I believe. Our next release will be 3.0.0 anyway.

Would you or @sindresorhus be willing to write some steps on how to fix the shrinkwrap issue if others are facing it? I've not used shrinkwrap personally.


Sorry if this inconvenienced anyone.

@sindresorhus

This comment has been minimized.

Show comment
Hide comment
@sindresorhus

sindresorhus Mar 28, 2016

Member

@hulbert The 2.2.0 version was broken. So I don't really see how not unpublishing would have helped when it's pinned?

Member

sindresorhus commented Mar 28, 2016

@hulbert The 2.2.0 version was broken. So I don't really see how not unpublishing would have helped when it's pinned?

@sindresorhus

This comment has been minimized.

Show comment
Hide comment
@sindresorhus

sindresorhus Mar 28, 2016

Member

Reopening for discovery.

I did a 2.2.1 patch release with the content of the old 2.1.0, just in case someone already upped it to ^2.2.0.

Member

sindresorhus commented Mar 28, 2016

Reopening for discovery.

I did a 2.2.1 patch release with the content of the old 2.1.0, just in case someone already upped it to ^2.2.0.

@sindresorhus sindresorhus reopened this Mar 28, 2016

@sindresorhus sindresorhus changed the title from 2.1.0 exports broken in 2.2.0 to [fixed] 2.1.0 exports broken in 2.2.0 Mar 28, 2016

@Qix-

This comment has been minimized.

Show comment
Hide comment
@Qix-

Qix- Mar 28, 2016

Member

Post Mortem

2.2.0 was actually released on Feb 21. Some breaking changes that were finally merged (ref #13) were sitting in master until we wanted to do a major version bump. However these changes were inadvertently released in 2.2.0 when the next release should have been 3.0.0 (minor bump instead of a major bump).

Since chalk wasn't affected by the otherwise breaking changes (this was by-design), but other projects that use ansi-styles might have been, the breaking release 2.2.0 went unnoticed until now.

This is also due to the fact the 'breaking' changes were not buggy code, but intentional changes meant for a major version bump, hence why tests still passed on Travis and thus why we were not notified of any breaking CI runs.

As Sindre mentioned, we went ahead and still did a 2.2.1 patch release despite the unpublishing of 2.2.0, for those users that had already bumped their package.json dependencies to ^2.2.0.

We acknowledge that the unpublishing of the broken version was unnecessary and a patch release would have been the preferable solution.

The root issue has been addressed internally (unrelated: externally, too) and won't be happening again in the future.


Maintainers that are facing errors related to ansi-styles after fixing broken code from the 2.2.0 release (i.e. users that upgraded to 2.2.0 and modified their code to use the new API) can do one of two things:

  • wait for the 3.0.0 release (your code should work when this happens)
  • temporarily change your dependency to "dependencies": {"ansi-styles": "chalk/ansi-styles"}

All other users experiencing errors related to ansi-styles should clear out their node_modules/ directory and re-run npm install - the problem should resolve itself.

We also recommend updating your dependencies to specify ansi-styles@^2.2.1, though this shouldn't be required.


If you use shrinkwrap, refer to #17.


For any other issues, you are definitely welcome to open a new ticket and I will personally see to it that it's handled in a timely manner.

Sorry again to anyone affected by this!

Member

Qix- commented Mar 28, 2016

Post Mortem

2.2.0 was actually released on Feb 21. Some breaking changes that were finally merged (ref #13) were sitting in master until we wanted to do a major version bump. However these changes were inadvertently released in 2.2.0 when the next release should have been 3.0.0 (minor bump instead of a major bump).

Since chalk wasn't affected by the otherwise breaking changes (this was by-design), but other projects that use ansi-styles might have been, the breaking release 2.2.0 went unnoticed until now.

This is also due to the fact the 'breaking' changes were not buggy code, but intentional changes meant for a major version bump, hence why tests still passed on Travis and thus why we were not notified of any breaking CI runs.

As Sindre mentioned, we went ahead and still did a 2.2.1 patch release despite the unpublishing of 2.2.0, for those users that had already bumped their package.json dependencies to ^2.2.0.

We acknowledge that the unpublishing of the broken version was unnecessary and a patch release would have been the preferable solution.

The root issue has been addressed internally (unrelated: externally, too) and won't be happening again in the future.


Maintainers that are facing errors related to ansi-styles after fixing broken code from the 2.2.0 release (i.e. users that upgraded to 2.2.0 and modified their code to use the new API) can do one of two things:

  • wait for the 3.0.0 release (your code should work when this happens)
  • temporarily change your dependency to "dependencies": {"ansi-styles": "chalk/ansi-styles"}

All other users experiencing errors related to ansi-styles should clear out their node_modules/ directory and re-run npm install - the problem should resolve itself.

We also recommend updating your dependencies to specify ansi-styles@^2.2.1, though this shouldn't be required.


If you use shrinkwrap, refer to #17.


For any other issues, you are definitely welcome to open a new ticket and I will personally see to it that it's handled in a timely manner.

Sorry again to anyone affected by this!

@laurelnaiad

This comment has been minimized.

Show comment
Hide comment
@laurelnaiad

laurelnaiad Mar 28, 2016

... and remember to do the same with jspm if you're using it, where this was hitting our project

jspm cache-clear

laurelnaiad commented Mar 28, 2016

... and remember to do the same with jspm if you're using it, where this was hitting our project

jspm cache-clear
@laurelnaiad

This comment has been minimized.

Show comment
Hide comment
@laurelnaiad

laurelnaiad Mar 28, 2016

unpublishing breaks a lot of things, though it was necessary in this case I believe

Why was it necessary? Why not publish the version you intended at 2.2.1, and let everybody move forward? If they're referencing a broken 2.2.0 then they can update. I'm not trying to be a PITA about it, just curious where the necessity of unpublishing comes in as opposed to just publishing 2.2.1.

laurelnaiad commented Mar 28, 2016

unpublishing breaks a lot of things, though it was necessary in this case I believe

Why was it necessary? Why not publish the version you intended at 2.2.1, and let everybody move forward? If they're referencing a broken 2.2.0 then they can update. I'm not trying to be a PITA about it, just curious where the necessity of unpublishing comes in as opposed to just publishing 2.2.1.

@Qix-

This comment has been minimized.

Show comment
Hide comment
@Qix-

Qix- Mar 28, 2016

Member

@laurelnaiad you're correct. Sindre and I discussed it more after the fact and we agree that a patch would have been better. A lesson for the future 👍

Member

Qix- commented Mar 28, 2016

@laurelnaiad you're correct. Sindre and I discussed it more after the fact and we agree that a patch would have been better. A lesson for the future 👍

@laurelnaiad

This comment has been minimized.

Show comment
Hide comment
@laurelnaiad

laurelnaiad Mar 28, 2016

Ok, it's not a big deal, just wanted to understand. Thanks! :)

laurelnaiad commented Mar 28, 2016

Ok, it's not a big deal, just wanted to understand. Thanks! :)

@Qix-

This comment has been minimized.

Show comment
Hide comment
@Qix-

Qix- Mar 28, 2016

Member

Thanks for the feedback 💃

Member

Qix- commented Mar 28, 2016

Thanks for the feedback 💃

@laurelnaiad

This comment has been minimized.

Show comment
Hide comment
@laurelnaiad

laurelnaiad Mar 28, 2016

the dancer emoticon should be bigger so we can see her fabulous red dress

laurelnaiad commented Mar 28, 2016

the dancer emoticon should be bigger so we can see her fabulous red dress

@tlrobinson

This comment has been minimized.

Show comment
Hide comment
@tlrobinson

tlrobinson Mar 28, 2016

Seriously, unpublishing should be reserved for exceptional cases. I'm not sure how you thought this was a good idea less than a week after the left-pad incident. I don't even know which of my dependencies uses this package, but it was working just fine until you unpublished 2.2.0.

tlrobinson commented Mar 28, 2016

Seriously, unpublishing should be reserved for exceptional cases. I'm not sure how you thought this was a good idea less than a week after the left-pad incident. I don't even know which of my dependencies uses this package, but it was working just fine until you unpublished 2.2.0.

@bradvogel

This comment has been minimized.

Show comment
Hide comment
@bradvogel

bradvogel Mar 28, 2016

PLEASE don't unpublish! This causes disasters downstream. NO, NO!

bradvogel commented Mar 28, 2016

PLEASE don't unpublish! This causes disasters downstream. NO, NO!

@Qix-

This comment has been minimized.

Show comment
Hide comment
@Qix-

Qix- Mar 28, 2016

Member

@tlrobinson We didn't unpublish the module, just that particular version. Quite different. It also wasn't working just fine. Chalk was working, sure - but that was because the breaking changes were designed to be compatible with how chalk was already using it.

However, some packages that used ansi-styles directly were broken.

We now know a patch version would have been the better alternative. However, we didn't know the extent of breaking that had been released until after the fire was contained. At the time, unpublish seemed like the correct course of action.

@bradvogel We're aware. 👍 Thank you!


If you're still facing issues after referring to the post-mortem, please open a new ticket.

I'll see to it personally that it is resolved quickly.

Member

Qix- commented Mar 28, 2016

@tlrobinson We didn't unpublish the module, just that particular version. Quite different. It also wasn't working just fine. Chalk was working, sure - but that was because the breaking changes were designed to be compatible with how chalk was already using it.

However, some packages that used ansi-styles directly were broken.

We now know a patch version would have been the better alternative. However, we didn't know the extent of breaking that had been released until after the fire was contained. At the time, unpublish seemed like the correct course of action.

@bradvogel We're aware. 👍 Thank you!


If you're still facing issues after referring to the post-mortem, please open a new ticket.

I'll see to it personally that it is resolved quickly.

@chalk chalk unlocked this conversation Mar 31, 2016

@Qix- Qix- removed the discovery label Apr 13, 2016

@Qix- Qix- closed this Apr 13, 2016

@Qix- Qix- referenced this issue Apr 20, 2016

Closed

Out of sync #110

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment