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

Update app template dependency versions #790

Merged
merged 2 commits into from
Apr 29, 2021

Conversation

bendemboski
Copy link
Contributor

I noticed that yarn.lock is out-of-date and running yarn would update it to add 0.39.1 versions of the embroider packages plus a bunch of their dependencies. I'm guessing the issue here isn't an out-of-date yarn.lock, but a merge oversight (?) resulting in app-template pointing at the 0.39.1 versions rather than 0.40.0, which I think means some of the tests weren't actually running against the latest version.

@embroider/shared-internals@^0.39.1 was added to yarn.lock, I believe because it's a transitive dependency of app-template's ember-auto-import dependency. I also updated the CI scripts to run yarn with --frozen-lockfile so yarn.lock being out-of-date will be caught by CI in the future.

I don't really know how scenario-tester works with workspaces, but I'm wondering if we should change app-template's @embroider/* versions in package.json to *. Assuming scenario-tester handles workspaces correctly, this would ensure that they always match and run against the local @embroider/* packages, and never fetch them from the registry and run against probably-older code.

If that doesn't make sense/work, then I suspect we'd want to remove app-template from the package workspaces and treat it as a standalone test fixture so its dependencies aren't mixed with the workspace's. As it stands, it might run against the local packages or it might run against some code fetched from the registry depending on the specifics of the different version strings, and that seems kinda non-deterministic for an automated testing environment.

@bendemboski
Copy link
Contributor Author

FWIW, I verified that setting the versions to * produces exactly the same yarn.lock as setting them to 0.40.0 (as expected). What I don't really know is if scenario-tester runs yarn in a copy of app-template outside of the workspace, meaning that it would fetch the latest from the registry rather than using the local workspace copies (which I think would still be better than using a potentially not-newest version). Or if it manually links the packages, in which case I think maybe making app-template not a workspace package would be the way to go.

The app template was still pointing to 0.39.1.
To make sure `yarn.lock` is up-to-date.
@ef4
Copy link
Contributor

ef4 commented Apr 29, 2021

so its dependencies aren't mixed with the workspace's

That is actually a deliberate feature of scenario-tester. In this case, you're correct that it was unintentional, and the normal release process is supposed to update these inside-the-monorepo dependencies to match, and we probably got it wrong after a merge. We should probably add a test to assert against that happening again.

For an example where we're intentionally making a package test against it's own earlier versions, see https://github.com/ef4/ember-auto-import/blob/d3059863fc84ed3c0520b64b232476a6022be945/test-scenarios/package.json#L25-L26

@ef4 ef4 merged commit 0f3359a into embroider-build:master Apr 29, 2021
@bendemboski
Copy link
Contributor Author

bendemboski commented Apr 29, 2021

Gotcha. And when these scenarios run, are the apps generated from the app template run in the workspace? i.e. are they sharing dependencies with /node_modules, or do they get put in some temp location and then do a full independent install of their dependencies? Because if it's the latter, then it seems like it would make sense to not have the app template as one of the workspace packages that installs the templates dependencies into the workspace, but instead just have it as a bunch of files that happen to resemble an application (i.e. a test fixture), no?

@ef4
Copy link
Contributor

ef4 commented Apr 29, 2021

are they sharing dependencies with /node_modules, or do they get put in some temp location and then do a full independent install of their dependencies?

It's neither of those exactly. We don't run any separate installs, that's pretty important, as putting yarn install inside your test suite is slow, painful, and error prone.

But we also don't just inherit from node_modules in an uncontrolled way. We do work in a temp directory, and the discovery and linking of dependencies is handled by scenario-tester (and it's dependency @ef4/fixturify-project).

So it's important that the app-template is a real workspace. We really do get the default set of dependencies from there. It also has the benefit of making things like ember-cli-update work that otherwise wouldn't.

@bendemboski
Copy link
Contributor Author

Got it. Thanks for helping me understand!

@ef4 ef4 added the internal label May 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants