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 yarn behaviour when scripts are failing #5497

Merged
merged 3 commits into from
Apr 5, 2018
Merged

Fix yarn behaviour when scripts are failing #5497

merged 3 commits into from
Apr 5, 2018

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