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

Add missing deps to @redwoodjs/forms package.json #4310

Merged
merged 4 commits into from
Jan 31, 2022

Conversation

jtoar
Copy link
Contributor

@jtoar jtoar commented Jan 31, 2022

While I was reviewing #4302, I ran yarn dlx @yarnpkg/doctor to see if anything was missing. There were a few things missing and a few things there that we don't need.

I also sorted the package.json file and removed some jest config because it wasn't necessary anymore; moreover, yarn dlx @yarnpkg/doctor doesn't like seeing node_modules in strings.

The output from yarn dlx @yarnpkg/doctor:

dom@evaM1 ~/p/r/r/p/forms (ds-update-forms-package.json)> yarn dlx @yarnpkg/doctor
➤ YN0000: ┌ Resolution step
➤ YN0000: └ Completed in 1s 827ms
➤ YN0000: ┌ Fetch step
➤ YN0013: │ util-deprecate@npm:1.0.2 can't be found in the cache and will be fetched from the remote registry
➤ YN0013: │ which@npm:2.0.2 can't be found in the cache and will be fetched from the remote registry
➤ YN0013: │ wrappy@npm:1.0.2 can't be found in the cache and will be fetched from the remote registry
➤ YN0013: │ yallist@npm:4.0.0 can't be found in the cache and will be fetched from the remote registry
➤ YN0013: │ yup@npm:0.32.11 can't be found in the cache and will be fetched from the remote registry
➤ YN0000: └ Completed in 0s 653ms
➤ YN0000: ┌ Link step
➤ YN0000: └ Completed in 0s 672ms
➤ YN0000: Done in 3s 173ms

➤ YN0000: Found 1 package(s) to process
➤ YN0000: For a grand total of 8 file(s) to validate

➤ YN0000: ┌ /Users/dom/prjcts/rw/redwood/packages/forms/package.json
➤ YN0000: │ User scripts prefixed with "pre" or "post" (like "prepublishOnly") will not be called in sequence anymore; prefer calling prologues and epilogues explicitly
➤ YN0000: └ Completed in 4s 193ms
➤ YN0000: ⠸ --------------------------------------------------------------------------------

➤ YN0000: Done with warnings in 4s 195ms

I'd like to fix the warning too in the future.

@jtoar jtoar added the release:chore This PR is a chore (means nothing for users) label Jan 31, 2022
@jtoar jtoar self-assigned this Jan 31, 2022
@thedavidprice
Copy link
Contributor

@jtoar Nice work here. Looks like you need to yarn dedupe locally.

I'd like to fix the warning too in the future.

^^ Are you referring to the User scripts prefixed with "pre" or "post" ...? If so, I'm aware of that one as well. The "pre" we use was originally added (I believe) for npm publish which is the underlying command run by lerna publish ... — we want to ensure packages are built before being published. But a couple thoughts here:

  • I think this might be redundant with lerna
  • given your recent yarn release, we might not need to be concerned

So the only time this could bit us is when I need to publish a package for the very first time, during which I will use npm publish directly on the package. But that's a simple case to manage.

@jtoar
Copy link
Contributor Author

jtoar commented Jan 31, 2022

@thedavidprice yeah that's the one. It's complaining about prepublishOnly. It wants us to use prepack instead: https://yarnpkg.com/advanced/rulebook#packages-should-use-the-prepack-script-to-generate-dist-files-before-publishing.

So my thoughts are twofold, both around CI:

  • I want to add yarn dlx @yarnpkg/doctor to CI
  • I want to add a check using yarn pack to make sure the tarball (i.e. what we publish) doesn't accidentally change unless we want it to

yarn pack calls the prepack script. So to start using yarn pack we should think about renaming our prepublishOnly scripts to prepack. Moreover, if we use yarn npm publish (not just npm publish), it'll call the prepack script before publishing too.

@thedavidprice
Copy link
Contributor

So my thoughts are twofold, both around CI:

I want to add yarn dlx @yarnpkg/doctor to CI
I want to add a check using yarn pack to make sure the tarball (i.e. what we publish) doesn't accidentally change unless we want it to
yarn pack calls the prepack script. So to start using yarn pack we should think about renaming our prepublishOnly scripts to prepack. Moreover, if we use yarn npm publish (not just npm publish), it'll call the prepack script before publishing too.

I'm all for it. I suggest merging this PR as is and handling those in follow-up PR, which we can validate during next release publishing cycle.

Sound good?

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.

🚀

@jtoar jtoar enabled auto-merge (squash) January 31, 2022 18:41
@jtoar jtoar merged commit c589783 into main Jan 31, 2022
@jtoar jtoar deleted the ds-update-forms-package.json branch January 31, 2022 18:54
@jtoar jtoar added this to the next-release milestone Jan 31, 2022
dac09 added a commit to dac09/redwood that referenced this pull request Feb 1, 2022
…ize-jest-config

* 'main' of github.com:redwoodjs/redwood: (46 commits)
  update contributing content (redwoodjs#4325)
  Upgrade yarn to 3.1.1 (redwoodjs#3919)
  Sort package.jsons (redwoodjs#4309)
  Update dependency esbuild to v0.14.16 (redwoodjs#4322)
  Add explanation and docs link in generated tests (redwoodjs#4218)
  Update dependency webpack to v5.68.0 (redwoodjs#4315)
  Update dependency esbuild to v0.14.15 (redwoodjs#4321)
  Update dependency cypress to v9.4.1 (redwoodjs#4319)
  Update dependency @typescript-eslint/parser to v5.10.2 (redwoodjs#4318)
  Pin dependencies (redwoodjs#4317)
  Pull proper keys for Yarn and npm (redwoodjs#4313)
  Add missing deps to @redwoodjs/forms package.json (redwoodjs#4310)
  Update dependency @typescript-eslint/eslint-plugin to v5.10.2 (redwoodjs#4314)
  Tweak release script (redwoodjs#4312)
  Update dependency @graphql-codegen/typescript-operations to v2.2.3 (redwoodjs#4311)
  Update dependency vscode-languageserver-textdocument to v1.0.4 (redwoodjs#4308)
  Update dependency copy-webpack-plugin to v10.2.4 (redwoodjs#4307)
  Update storybook monorepo to v6.4.17 (redwoodjs#4306)
  Update release script (redwoodjs#4305)
  Update dependency msw to v0.36.8 (redwoodjs#4301)
  ...
@thedavidprice thedavidprice modified the milestones: next-release, v0.44.0 Feb 3, 2022
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
No open projects
Status: Archived
Development

Successfully merging this pull request may close these issues.

None yet

3 participants