Skip to content

Conversation

@sth
Copy link
Contributor

@sth sth commented Mar 10, 2018

Summary
Fix for #5451, #5457.

The test case in the first commit fails for pre-node5 builds, in the way described in the links issues. The problem are classes deriving from Error, which are problematic in ES5 (babel). Adding babel-plugin-transform-builtin-extend fixes the issue in this case.

Why now?
The error surfaced because the current yarn build published on npm is a pre-node5 build while for example v1.3.1 seems to have been created by the more modern build configuration. This can be seen by checking if code like "class A extends B" got compiled to ES5 or not.

@sth sth changed the title Add test case to check behaviour for failing scripts Add test case to check behaviour of failing scripts Mar 10, 2018
Added babel-plugin-transform-builtin-extend
@buildsize
Copy link

buildsize bot commented Mar 10, 2018

This change will increase the build size from 10.51 MB to 10.52 MB, an increase of 9.31 KB (0%)

File name Previous Size New Size Change
yarn-[version].noarch.rpm 910.69 KB 911.87 KB 1.18 KB (0%)
yarn-[version].js 3.96 MB 3.96 MB 41 bytes (0%)
yarn-legacy-[version].js 4.11 MB 4.11 MB 6.19 KB (0%)
yarn-v[version].tar.gz 916.08 KB 917.1 KB 1.03 KB (0%)
yarn_[version]all.deb 676.31 KB 677.19 KB 900 bytes (0%)

@sth sth changed the title Add test case to check behaviour of failing scripts Fix yarn behaviour when scripts are failing Mar 10, 2018
@arcanis
Copy link
Member

arcanis commented Mar 15, 2018

The error surfaced because the current yarn build published on npm is a pre-node5 build while for example v1.3.1 seems to have been created by the more modern build configuration.

That looks weird :/ cc @BYK @Daniel15 did we change something recently?

@rally25rs
Copy link
Contributor

wow, nice find. 👏 I never would have guessed that err instanceof MessageError wouldn't work. I'm curious what happened in the build process, but either way, it looks like this babel plugin fixes it.

@Daniel15
Copy link
Member

Daniel15 commented Apr 5, 2018

It looks like we haven't changed the npm publishing code in quite a while: https://github.com/yarnpkg/yarn/blame/master/scripts/update-npm.sh. Maybe the build config changed, but I'm not sure.

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.

4 participants