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

chore(rwfw): working refactor, make project:sync ignore more files #8579

Merged
merged 12 commits into from
Jun 13, 2023

Conversation

jtoar
Copy link
Contributor

@jtoar jtoar commented Jun 9, 2023

When I clean the framework (git clean -fxd -e .env) so that build has to happen from scratch instead of pulling from the Nx cache, I can reproduce yarn rwfw project:sync failing with:

rwfw  /<path>/redwood/packages/cli-packages/storybook/meta.json
node:fs:601
  handleErrorFromBinding(ctx);
  ^

Error: ENOENT: no such file or directory, open '/<path>/redwood/packages/cli-packages/package.json'
    at Object.openSync (node:fs:601:3)
    at Object.readFileSync (node:fs:469:35)
    at packageJsonName (file:///<path>/redwood/tasks/framework-tools/lib/framework.mjs:146:24)
    at FSWatcher.<anonymous> (file:///<path>/redwood/tasks/framework-tools/frameworkSyncToProject.mjs:100:25)
    at FSWatcher.emit (node:events:513:28)
    at FSWatcher.emitWithAll (/<path>/redwood/node_modules/chokidar/index.js:541:32)
    at awfEmit (/<path>/redwood/node_modules/chokidar/index.js:607:14)
    at /<path>/redwood/node_modules/chokidar/index.js:728:9
    at FSReqCallback.oncomplete (node:fs:209:5) {
  errno: -2,
  syscall: 'open',
  code: 'ENOENT',
  path: '/<path>/redwood/packages/cli-packages/package.json'
}

I'm not sure exactly why this is happening, but the code for yarn rwfw project:sync could be stricter about what it watches, how it finds package.jsons, and a few other things. So far these changes seem to fix it it for me, but it doesn't seem like that's true for everyone yet.

Working list of changes

  • cleanPackages stopped working at some point; I've fixed it for cleaning all packages by keeping it simple and just calling the script in the root package.json. And on a per-packge basis, I just call rimraf
  • I've move the logic in the ready handler. Now it just happens before we call watch. I feel like the less chokidar, the better, but let me know if there's a reason the ready handler is better
  • I've renamed a bunch of functions. This is a stylistic preference of mine; most of the functions were named as if they were constants. And some names for local vars were too vague
  • I still want to do work on the fixBins logic. I think it's happening too often
  • I've added some options; too many probably. Sometimes when yarn rwfw project:sync fails, you don't want to have to clean everything. I could probably make this easier by having a set-up-for-watch flag or something like that, to do all the steps before watching. Also, yargs aliases are making no sense right now and are duplicating properties, so figuring that out

@jtoar jtoar added the release:chore This PR is a chore (means nothing for users) label Jun 9, 2023
@jtoar jtoar force-pushed the ds-tasks/fix-project-sync branch from 673fefe to 65f9a97 Compare June 9, 2023 23:49
@@ -170,7 +170,7 @@ export function buildPackages(packages = frameworkPkgJsonFiles()) {
const packageNames = packages.map(packageJsonName)
execa.sync(
'yarn lerna run build',
['--parallel', `--scope={${packageNames.join(',') + ','}}`],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nx issues a warning that this option doesn't do anything anymore

@jtoar jtoar mentioned this pull request Jun 10, 2023
1 task
@jtoar jtoar force-pushed the ds-tasks/fix-project-sync branch 2 times, most recently from 5ef06f1 to 4ff1e87 Compare June 10, 2023 02:59
@jtoar jtoar force-pushed the ds-tasks/fix-project-sync branch from 4ff1e87 to 84ce669 Compare June 10, 2023 03:57
@jtoar jtoar marked this pull request as ready for review June 10, 2023 18:09
@thedavidprice thedavidprice self-requested a review June 12, 2023 23:13
Copy link
Contributor

@thedavidprice thedavidprice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Local QA == ✅

Steps:

  • create redwood app for new project at 5.3.0
  • check out PR branch
  • git clean -fxd -e .env
  • yarn
    from Project directory
  • yarn rwfw project:sync
Screenshot 2023-06-12 at 4 10 03 PM

Copy link
Contributor

@thedavidprice thedavidprice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, blast. 👎 Per request to also test file changes over here in #8580, I now get the error message after sync is successful but I then edit and save a file. To reproduce:

  • yarn rwfw project:sync (successful)
  • edit packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts

Fails with the error message below.

rwfw  Waiting for changes
-------------------------------------------------------------------------------------------------------------
rwfw  /Users/price/Repos/redwoodjs-redwood/packages/auth-providers/dbAuth/api/src/DbAuthHandler.ts changed
rwfw  Restoring the Redwood project's package.json...

To get your project back to its original state...
- undo the changes to project's your yarn.lock file
- remove your project's node_modules directory
- run 'yarn install'

rwfw  Closing the watcher...
node:fs:601
  handleErrorFromBinding(ctx);
  ^

Error: /Users/price/Repos/redwoodjs-redwood/packages/auth-providers/package.json: ENOENT: no such file or directory, open '/Users/price/Repos/redwoodjs-redwood/packages/auth-providers/package.json'
    at Object.openSync (node:fs:601:3)
    at Object.readFileSync (node:fs:469:35)
    at Object.readFileSync (/Users/price/Repos/redwoodjs-redwood/node_modules/jsonfile/index.js:50:22)
    at getPackageJsonName (file:///Users/price/Repos/redwoodjs-redwood/tasks/framework-tools/lib/framework.mjs:161:13)
    at FSWatcher.<anonymous> (file:///Users/price/Repos/redwoodjs-redwood/tasks/framework-tools/frameworkSyncToProject.mjs:234:25)
    at FSWatcher.emit (node:events:513:28)
    at FSWatcher.emitWithAll (/Users/price/Repos/redwoodjs-redwood/node_modules/chokidar/index.js:541:32)
    at awfEmit (/Users/price/Repos/redwoodjs-redwood/node_modules/chokidar/index.js:607:14)
    at /Users/price/Repos/redwoodjs-redwood/node_modules/chokidar/index.js:728:9
    at FSReqCallback.oncomplete (node:fs:209:5) {
  errno: -2,
  syscall: 'open',
  code: 'ENOENT',
  path: '/Users/price/Repos/redwoodjs-redwood/packages/auth-providers/package.json'
}

Node.js v18.16.0

@jtoar
Copy link
Contributor Author

jtoar commented Jun 13, 2023

Thanks @thedavidprice and @arimendelow, I realize there's an error in the resolvePackageJsonPath now

@jtoar
Copy link
Contributor Author

jtoar commented Jun 13, 2023

@arimendelow I think I've addressed the issue if you wouldn't mind trying again whenever you have time

@arimendelow
Copy link
Contributor

ah it works!!! thanks @jtoar 🙂 i really appreciate how quickly you fixed this

Screenshot 2023-06-12 at 10 35 50 PM

@thedavidprice thedavidprice merged commit 2a8381e into main Jun 13, 2023
@thedavidprice thedavidprice deleted the ds-tasks/fix-project-sync branch June 13, 2023 19:40
@redwoodjs-bot redwoodjs-bot bot added this to the next-release milestone Jun 13, 2023
jtoar added a commit that referenced this pull request Jun 22, 2023
…8579)

* working refactor, make project:sync ignore more

* working refactor, make project:sync ignore more

* consolidate flags, fix resolvePackageJsonPath

* remove caching for now

* improve comments

* actually fix edge case for web/apollo etc

* unlink before trying to symlink

* handle broken symlinks better

* style: spacing

---------
@jtoar jtoar modified the milestones: next-release, v5.4.0 Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:chore This PR is a chore (means nothing for users)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants