-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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] improve tarball insertion #6808
Conversation
Asset Size Report for 67dc107 EmberData has not changed in sizeIf any packages had changed sizes they would be listed here. Changeset
Full Asset Analysis
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you confirmed that all of our partner tests use yarn
?
// | ||
// For this reason we don't trust the lock file | ||
// we also can't trust the cache | ||
execExternal(`yarn install --no-lockfile --cache-folder=tmp/yarn-cache`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, npm and yarn aren't drop in replacements for each other. There are quite a few bizarre inconsistencies (e.g. around using file:
or link:
references for deps/devDeps).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before we were using npm regardless of whether the partner used yarn or npm because yarn was failing to extract the tarballs. This appears to have been fixed! However, I discovered a bug with npm wherein both during link and when installing from tarball nested dependencies specifying ember-data wont resolve to our tarball even if it is "in range" (I think this may be due to it being an alpha).
So we can't use npm safely, so using yarn+resolutions (to force the nested deps) always seems like the way to go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, tarballs can never be "in range". They are always treated as "exotic" and the system cannot make assumptions about greater than / less than.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rwjblue all the more reason to not install with npm unless they’ve added support for resolutions that I don’t know about (based on testing they don’t seem to respect them)
Asset Size Report for 77e97f5 EmberData has not changed in sizeIf any packages had changed sizes they would be listed here. Changeset
Full Asset Analysis
|
Asset Size Report for 0952f7c EmberData has not changed in sizeIf any packages had changed sizes they would be listed here. Changeset
Full Asset Analysis
|
Since we wrote this script, yarn has improved and we might now be able to rely on its use of resolutions. Resolutions seems to be the only way to force partners that have addons with dependencies on ember-data to use the version we want, otherwise things like
ember-cli-addon-docs
install a stable release as well.