-
Notifications
You must be signed in to change notification settings - Fork 1k
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
feat(cli): lazy install data migrate and make it runnable on its own #8572
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jtoar
commented
Jun 9, 2023
jtoar
commented
Jun 9, 2023
jtoar
commented
Jun 9, 2023
jtoar
commented
Jun 9, 2023
1 task
jtoar
force-pushed
the
ds-cli-plugins/data-migrate
branch
from
August 22, 2023 00:47
3edc1b0
to
1aa7be3
Compare
jtoar
force-pushed
the
ds-cli-plugins/data-migrate
branch
19 times, most recently
from
August 29, 2023 04:01
8e174c7
to
82fa032
Compare
jtoar
force-pushed
the
ds-cli-plugins/data-migrate
branch
from
August 29, 2023 04:40
a7033de
to
c0174a0
Compare
jtoar
added a commit
that referenced
this pull request
Aug 29, 2023
Canary publish was erroring out on there being no build:js script. Follow up to #8572.
edit: I had not built locally after pulling in these changes, which is why I was seeing the below error.
$ yarn lint:fix
Oops! Something went wrong! :(
ESLint: 8.46.0
Error: Cannot read config file: /code/redwood/packages/eslint-config/index.js
Error: Cannot find module '/code/redwood/node_modules/@redwoodjs/babel-config/dist/index.js'
Referenced from: /code/redwood/packages/create-redwood-app/templates/js/package.json
at createEsmNotFoundErr (node:internal/modules/cjs/loader:1098:15)
at finalizeEsmResolution (node:internal/modules/cjs/loader:1091:15)
at resolveExports (node:internal/modules/cjs/loader:567:14)
at Module._findPath (node:internal/modules/cjs/loader:636:31)
at Module._resolveFilename (node:internal/modules/cjs/loader:1063:27)
at Module._load (node:internal/modules/cjs/loader:922:27)
at Module.require (node:internal/modules/cjs/loader:1143:19)
at require (node:internal/modules/cjs/helpers:121:18)
at Object.<anonymous> (/code/redwood/packages/eslint-config/index.js:9:5)
at Module._compile (node:internal/modules/cjs/loader:1256:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1310:10)
at Module.load (node:internal/modules/cjs/loader:1119:32)
at Module._load (node:internal/modules/cjs/loader:960:12)
at Module.require (node:internal/modules/cjs/loader:1143:19)
at module.exports [as default] (/code/redwood/node_modules/import-fresh/index.js:32:59)
at loadJSConfigFile (/code/redwood/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2574:47)
at loadConfigFile (/code/redwood/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2658:20)
at ConfigArrayFactory._loadConfigData (/code/redwood/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2975:42)
at ConfigArrayFactory._loadExtendedShareableConfig (/code/redwood/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3276:21)
at ConfigArrayFactory._loadExtends (/code/redwood/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3147:25)
at ConfigArrayFactory._normalizeObjectConfigDataBody (/code/redwood/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3086:25)
at _normalizeObjectConfigDataBody.next (<anonymous>)
at ConfigArrayFactory._normalizeObjectConfigData (/code/redwood/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3031:20)
at _normalizeObjectConfigData.next (<anonymous>)
at ConfigArrayFactory.loadInDirectory (/code/redwood/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2877:28)
at CascadingConfigArrayFactory._loadConfigInAncestors (/code/redwood/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3860:46)
at CascadingConfigArrayFactory.getConfigArrayForFile (/code/redwood/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3781:18)
at FileEnumerator._iterateFilesRecursive (/code/redwood/node_modules/eslint/lib/cli-engine/file-enumerator.js:450:49)
at _iterateFilesRecursive.next (<anonymous>)
at FileEnumerator._iterateFilesRecursive (/code/redwood/node_modules/eslint/lib/cli-engine/file-enumerator.js:497:33)
at _iterateFilesRecursive.next (<anonymous>)
at FileEnumerator._iterateFilesRecursive (/code/redwood/node_modules/eslint/lib/cli-engine/file-enumerator.js:497:33)
at _iterateFilesRecursive.next (<anonymous>)
at FileEnumerator._iterateFilesRecursive (/code/redwood/node_modules/eslint/lib/cli-engine/file-enumerator.js:497:33)
at _iterateFilesRecursive.next (<anonymous>)
at FileEnumerator.iterateFiles (/code/redwood/node_modules/eslint/lib/cli-engine/file-enumerator.js:299:49)
at iterateFiles.next (<anonymous>)
at CLIEngine.executeOnFiles (/code/redwood/node_modules/eslint/lib/cli-engine/cli-engine.js:797:48)
at ESLint.lintFiles (/code/redwood/node_modules/eslint/lib/eslint/eslint.js:551:23)
at Object.execute (/code/redwood/node_modules/eslint/lib/cli.js:391:36)
at async main (/code/redwood/node_modules/eslint/bin/eslint.js:135:24) |
jtoar
added a commit
that referenced
this pull request
Aug 29, 2023
npx `@redwoodjs/cli-data-migrate` was erroring out on a missing dependency, `@redwoodjs/cli-helpers`. Follow up to #8572.
This was referenced Aug 29, 2023
jtoar
added a commit
that referenced
this pull request
Aug 30, 2023
Follow up to #8572. The latest suite of fixes to the new `@redwoodjs/cli-data-migrate` package. The previous were: - c27f318 fix(data-migrate): fix canary publishing - 2d6b93d fix(data-migrate): add missing dependency `@redwoodjs/cli-helpers` @Josh-Walker-GM I'm going to take a shot at it, but I'll probably need your help making sure this is properly plugged into the CLI.
jtoar
added a commit
that referenced
this pull request
Aug 31, 2023
…9085) Follow up to #8572. Even if I remove `@redwoodjs/babel-config`'s dependency on `@redwoodjs/structure`, both `@redwoodjs/cli-helpers` and `@redwoodjs/telemetry` depend on it. I have to choose my battles here, so it feels like we should just remove cli-helpers and telemetry from this cli-data-migrate for now.
jtoar
added a commit
that referenced
this pull request
Sep 2, 2023
…8572) This PR makes the data migrate CLI command (`yarn rw data-migrate`) a plugin like #8454 did for storybook. But this PR goes a little further, also adding a bin that runs the data migrate's `up` command. I don't think it makes sense for all of Redwood's CLI commands to do this, but it does for some, and data migrate is one since it's an important part of Docker deploys. - This PR extracts the Babel config out of `@redwoodjs/internal` into its own package, `@redwoodjs/babel-config`. If I didn't do this, `@redwoodjs/cli-data-migrate` would depend on `@redwoodjs/internal` because it needs the `registerApiSideBabelHook` function, which would make it's package size unacceptable - Even without internal, the new package has an unfortunate dependency on `@redwoodjs/structure` that I'm not going to try to refactor in this PR - Just noting that `@redwoodjs/structure` doesn't depend on any other Redwood packages besides `@redwoodjs/project-config`, so I'm sure we could make it smaller if we took the time to focus on it - The Babel config in `@redwoodjs/internal` had a dependency on the `typescript` package that I had to refactor as well - The Babel config needs a lot of TLC, but strictly speaking it'd be breaking so I'm holding off on that for now again. I'll open another PR after this one that has those changes
jtoar
added a commit
that referenced
this pull request
Sep 2, 2023
Canary publish was erroring out on there being no build:js script. Follow up to #8572.
jtoar
added a commit
that referenced
this pull request
Sep 2, 2023
npx `@redwoodjs/cli-data-migrate` was erroring out on a missing dependency, `@redwoodjs/cli-helpers`. Follow up to #8572.
jtoar
added a commit
that referenced
this pull request
Sep 2, 2023
Follow up to #8572. The latest suite of fixes to the new `@redwoodjs/cli-data-migrate` package. The previous were: - c27f318 fix(data-migrate): fix canary publishing - 2d6b93d fix(data-migrate): add missing dependency `@redwoodjs/cli-helpers` @Josh-Walker-GM I'm going to take a shot at it, but I'll probably need your help making sure this is properly plugged into the CLI.
jtoar
added a commit
that referenced
this pull request
Sep 2, 2023
…9085) Follow up to #8572. Even if I remove `@redwoodjs/babel-config`'s dependency on `@redwoodjs/structure`, both `@redwoodjs/cli-helpers` and `@redwoodjs/telemetry` depend on it. I have to choose my battles here, so it feels like we should just remove cli-helpers and telemetry from this cli-data-migrate for now.
jtoar
added a commit
that referenced
this pull request
Oct 6, 2023
Adds an experimental setup command to add a Dockerfile and compose files: ``` yarn rw exp setup-docker ``` The Dockerfile is near enough complete that we should release it experimentally so that we can get feedback from the community. I've successfully deployed it to Coherence and Fly. As far as I can tell, Render has half support. The api side runs just fine, but the web side has a static runtime, and I'm not sure how to get it to take the files from the web build stage. There's a known sticking point here: seeding and data migrations. I understand the problem clearly now. On all providers, seed and data migrations run using the image built from targeting api serve. There's only production dependencies in that image—`yarn rw exec` won't work for a number of reasons, but the main one is that `@redwoodjs/internal` isn't around. It's looking like we'll have to move `yarn rw exec` into it's own package and that we'll have to finish #8572. The docs are there but are very much a WIP.
jtoar
added a commit
that referenced
this pull request
Oct 8, 2023
Adds an experimental setup command to add a Dockerfile and compose files: ``` yarn rw exp setup-docker ``` The Dockerfile is near enough complete that we should release it experimentally so that we can get feedback from the community. I've successfully deployed it to Coherence and Fly. As far as I can tell, Render has half support. The api side runs just fine, but the web side has a static runtime, and I'm not sure how to get it to take the files from the web build stage. There's a known sticking point here: seeding and data migrations. I understand the problem clearly now. On all providers, seed and data migrations run using the image built from targeting api serve. There's only production dependencies in that image—`yarn rw exec` won't work for a number of reasons, but the main one is that `@redwoodjs/internal` isn't around. It's looking like we'll have to move `yarn rw exec` into it's own package and that we'll have to finish #8572. The docs are there but are very much a WIP.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR makes the data migrate CLI command (
yarn rw data-migrate
) a plugin like #8454 did for storybook. But this PR goes a little further, also adding a bin that runs the data migrate'sup
command. I don't think it makes sense for all of Redwood's CLI commands to do this, but it does for some, and data migrate is one since it's an important part of Docker deploys.@redwoodjs/internal
into its own package,@redwoodjs/babel-config
. If I didn't do this,@redwoodjs/cli-data-migrate
would depend on@redwoodjs/internal
because it needs theregisterApiSideBabelHook
function, which would make it's package size unacceptable@redwoodjs/structure
that I'm not going to try to refactor in this PR@redwoodjs/structure
doesn't depend on any other Redwood packages besides@redwoodjs/project-config
, so I'm sure we could make it smaller if we took the time to focus on it@redwoodjs/internal
had a dependency on thetypescript
package that I had to refactor as well