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

Publish Canary Builds #6010

Closed
ryanto opened this issue Apr 10, 2019 · 13 comments
Closed

Publish Canary Builds #6010

ryanto opened this issue Apr 10, 2019 · 13 comments

Comments

@ryanto
Copy link
Contributor

ryanto commented Apr 10, 2019

Hey there!

Currently in my ember-try config I'm testing against Ember Data's github repo as a way to simulate a canary scenario.

// config/ember-try.js
{
  name: 'ember-canary',
  npm: {
    devDependencies: {
      'ember-source': urls[2],
       'ember-data': 'emberjs/data'
    }
  }
}

I noticed that Ember Data recently switched over to a mono repo, which means no more npm installs from github.

I'm wondering if a canary build of Ember Data gets published anywhere (similar to what ember-source does)?

Also, looking for advice on how to best approach this problem, since there's probably a better way that I'm not aware of.

As always, thanks!

@runspired
Copy link
Contributor

@ryanto we are working to setup similar canary publishing as yes, we require a publishing step now prior to consumption

@rwjblue
Copy link
Member

rwjblue commented Apr 10, 2019

Might be able to do something like:

{
  name: 'ember-canary',
  npm: {
    devDependencies: {
      'ember-source': urls[2],
      'ember-data-mono': 'emberjs/data',
      'ember-data': 'file:./node_modules/ember-data-mono/packages/ember-data',
    },
    resolutions: {
      'ember-data/@ember-data/store': 'file:./node_modules/ember-data-mono/packages/store',
      'ember-data/@ember-data/adapter': 'file:./node_modules/ember-data-mono/packages/adapter',
      'ember-data/@ember-data/model': 'file:./node_modules/ember-data-mono/packages/model',
      'ember-data/@ember-data/serializer': 'file:./node_modules/ember-data-mono/packages/serializer',
    }
  }
}

Kinda just spit-ballin' though...

@runspired runspired changed the title How to test against monorepo? Publish Canary Builds Apr 11, 2019
@CvX
Copy link
Contributor

CvX commented Apr 18, 2019

@rwjblue I've tried out your idea in an app. There are two issues that prevent it from working:

  1. It seems that yarn is resolving resolutions before installing the dependencies (so you'd have to add that field after changing devDependencies and running yarn install):
yarn install v1.15.2
[1/5] Validating package.json...
[2/5] Resolving packages...
error Package "" refers to a non-existing file '"/(…)/node_modules/ember-data-mono/packages/store"'.
  1. The emberjs/data repo can't be added as a dependency because its root package.json doesn't have a version:
yarn install v1.15.2
[1/5] Validating package.json...
[2/5] Resolving packages...
error Can't add "ember-data-mono": invalid package version undefined.

@runspired
Copy link
Contributor

runspired commented Apr 22, 2019

The ember-data team needs to begin publishing canary builds. Previously we guided folks to use the git resource emberjs/data#master to test canary, but with the introduction of packages published from a monorepo this guidance no longer works.

After a bit of discussion, @rwjblue and I feel it may be time to begin publishing canary builds to npm using a canary dist-tag, and we are interested in soliciting input in this area.

Note that the data team needs to begin publishing canary builds in some form very soon, and we would like to avoid asking our consumers to change how they consume canary more than once.

Some background: previous complaints included

  • versioning of canary is hard (lerna publish in our new flow makes this much easier)
  • making them available on the registry reduces some barriers to usage
  • the registry returns the results of every publish when accessing https://registry.npmjs.org/<package-name> directly. (We believe yarn and npm are smart enough now this isn’t a big concern and npm view <package-name> does not return all packages.

@wagenet
Copy link
Member

wagenet commented Apr 22, 2019

@runspired I can't think of any objections to a canary tag in npm, but I also admit to not being aware of all the nuances involved.

@stefanpenner
Copy link
Member

the registry returns the results of every publish when accessing https://registry.npmjs.org/ directly. (We believe yarn and npm are smart enough now this isn’t a big concern and npm view does not return all packages.

Last I checked, when npm/yarn do the actual resolution, they still must download the complete entry, are we concerned this entry will grow? If this contained 6000+ canary versions, would this impact dependency installation time?


On another note, was this already discussed as part of the effort to convert to a mono-repo? If so, could that discussion be linked?

@rwjblue
Copy link
Member

rwjblue commented Apr 23, 2019

If this contained 6000+ canary versions, would this impact dependency installation time?

Yes, overall time would increase (a pretty small amount of time as a percentage of the overall time for the command) but only for the person regenerating the yarn.lock (e.g. when updating things in package.json, or adding a new package to the package.json) and not generally for all users doing an arbitrary yarn (when there are no changes).

@runspired
Copy link
Contributor

@stefanpenner

was this already discussed as part of the effort to convert to a mono-repo?

it was not previously discussed. There was an assumption we'd do something similar to ember-source but unlike ember-source we have actual packages and so that process is a bit more complicated. In light of that consideration, we decided to explore whether the simple-thing of "just use the ecosystem" wouldn't be the best approach.

are we concerned this entry will grow? If this contained 6000+ canary versions, would this impact dependency installation time?

Typescript has over a thousand at this time and the a precursory check seems to suggest this isn't a major issue.

Granted the below smoke-test would resolve to latest and so yarn and npm may be doing things more intelligently than asking for the list of all packages, as no resolutions are involved.

For a project with no dependencies and no lock file (empty package.json) we find comparing installation of typescript with installation of flow-bin to be similar:

  • npm install --save typescript 1.70s user 0.41s system 110% cpu 1.910 total
  • yarn add typescript 1.47s user 0.53s system 125% cpu 1.604 total
  • npm install --save flow-bin 2.45s user 0.63s system 99% cpu 3.105 total
  • yarn add flow-bin 1.78s user 0.73s system 97% cpu 2.572 total

For reference, npm view reports the following for these packages:

  • typescript is 44MB and has 1264 versions
  • flow-bin is 68.4MB and has 122 versions
time npm install --save typescript
npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN [email protected] No description
npm WARN [email protected] No repository field.

+ [email protected]
added 1 package from 1 contributor and audited 1 package in 1.307s
found 0 vulnerabilities

npm install --save typescript  1.70s user 0.41s system 110% cpu 1.910 total
 ~/lnkd/test-npm   master ●  git add -A && git reset --hard HEAD
HEAD is now at 16b13f7 fix
 ~/lnkd/test-npm   master  time yarn add typescript
yarn add v1.15.2
(node:8129) ExperimentalWarning: The fs.promises API is experimental
info No lockfile found.
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
[3/4] 🔗  Linking dependencies...
[4/4] 🔨  Building fresh packages...

success Saved lockfile.
success Saved 1 new dependency.
info Direct dependencies
└─ [email protected]
info All dependencies
└─ [email protected]
✨  Done in 1.29s.
yarn add typescript  1.47s user 0.53s system 125% cpu 1.604 total
 ~/lnkd/test-npm   master ●  git add -A && git reset --hard HEAD
HEAD is now at 16b13f7 fix
 ~/lnkd/test-npm   master  time npm install --save flow-bin
npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN [email protected] No description
npm WARN [email protected] No repository field.

+ [email protected]
added 1 package from 1 contributor and audited 1 package in 2.54s
found 0 vulnerabilities

npm install --save flow-bin  2.45s user 0.63s system 99% cpu 3.105 total
 ~/lnkd/test-npm   master ●  git add -A && git reset --hard HEAD
HEAD is now at 16b13f7 fix
 ~/lnkd/test-npm   master  time yarn add flow-bin
yarn add v1.15.2
(node:8735) ExperimentalWarning: The fs.promises API is experimental
info No lockfile found.
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
[3/4] 🔗  Linking dependencies...
[4/4] 🔨  Building fresh packages...
success Saved lockfile.
success Saved 1 new dependency.
info Direct dependencies
└─ [email protected]
info All dependencies
└─ [email protected]
✨  Done in 2.27s.
yarn add flow-bin  1.78s user 0.73s system 97% cpu 2.572 total

@runspired
Copy link
Contributor

After discussing with @krisselden @kategengler and @stefanpenner out-of-band there appears to be broad agreement that publishing to npm is desirable but that we should mitigate the growth of published package versions by

  • only publishing one canary a day
  • not publishing a canary on days there are no changes since the last canary

This is in-line with my own understanding of what we wanted to achieve (a daily publish, not a per-commit publish).

@runspired
Copy link
Contributor

resolved by #6074

We now publish a nightly canary and weekly beta to npm. You can install these using

yarn add ember-data@beta and yarn add ember-data@canary respectively.

@richard-viney
Copy link

Trying out the new beta release and it's working well but am seeing the following messages:

'@ember-data/store/-private' is imported by ../../../../var/folders/9d/25gd3x292fx1tpwfmhtj7xxw0000gp/T/broccoli-63816RKVeV4BPx25P/cache-292-rollup/build/-private/index.js, but could not be resolved – treating it as an external dependency '@ember-data/store' is imported by ../../../../var/folders/9d/25gd3x292fx1tpwfmhtj7xxw0000gp/T/broccoli-63816RKVeV4BPx25P/cache-292-rollup/build/-private/index.js, but could not be resolved – treating it as an external dependency '@ember-data/model' is imported by ../../../../var/folders/9d/25gd3x292fx1tpwfmhtj7xxw0000gp/T/broccoli-63816RKVeV4BPx25P/cache-292-rollup/build/-private/system/debug/debug-adapter.js, but could not be resolved – treating it as an external dependency

The app seems to be working fine, but is there anything to be done about the above?

@rwjblue
Copy link
Member

rwjblue commented May 2, 2019

Yes, we need to fix the warnings. @richard-viney mind reporting a specific issue (after double checking that one isn't already tracking this)?

@runspired
Copy link
Contributor

@richard-viney @rwjblue ostensibly this is a subtask of fixing our rollup step which has an issue already

Turbo87 added a commit to Turbo87/ember-simple-auth that referenced this issue Jul 4, 2019
`ember-data` is a monorepo now, so these kinds of version constraint declarations won't work anymore. see emberjs/data#6010
marcoow pushed a commit to mainmatter/ember-simple-auth that referenced this issue Jul 4, 2019
* ember-try: Fix broken `ember-data` dependency declarations

`ember-data` is a monorepo now, so these kinds of version constraint declarations won't work anymore. see emberjs/data#6010

* CI: Use default dependencies for default scenarios

The `latest` version of ember-data (v3.11) is using async/await in Node.js code, but since we run CI on Node.js 6 it does not support that yet. This means we will have to use an older version of ember-data for now until we drop support for Node.js 6.

* CI: Allow failures for `ember-beta` scenario

see previous commit about async/await usage in recent ember-data releases

* package.json: Pin `cssstyle` to v1.2.x

see jsdom/cssstyle#95

* package.json: Pin `ember-data` to v3.7.x

Using `^3.0.0` currently resolves to v3.11.x which is not compatible with Node.js 6 anymore. v3.8.x and above have some other issues so we will use v3.7.x until we figure those out.

* Revert "Build(deps-dev): bump mermaid from 7.1.2 to 8.0.0 (#1765)"

This reverts commit 95ae7cb.

Some dependencies of `mermaid@8` are incompatible with Node.js 6
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

No branches or pull requests

7 participants