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

fix(deps): remove react types packages from @redwoodjs/testing dependencies #10020

Merged
merged 1 commit into from
Feb 16, 2024

Conversation

jtoar
Copy link
Contributor

@jtoar jtoar commented Feb 15, 2024

Have been thinking about this one on and off for a while now and I don't think any of our packages should have the react types packages as dependencies if the web workspace in apps is going to have them.

More generally, if a Redwood app is going to explicitly depend on a package that one of our framework packages also depends on, one of them should go or use the '*' specifier.

I ran into an issue related to this a day or two ago with the deploy target CI providers. It was easily fixed if you knew what to look for. Unpinning them was a step forward cause yarn can sometimes make it work if you run dedupe or at worst edit or regenerate the lock file. But most people don't know what to look for and we shouldn't expect them to.

The reason we didn't do this before (see the original comment in #9727) was that the mailer depends on the react types packages but doesn't explicitly list them as dependencies. Well, it can still get the react types packages implicitly from node_modules anyway cause the web workspace puts them there.

@jtoar jtoar added the release:fix This PR is a fix label Feb 15, 2024
@jtoar jtoar added this to the v7.0.0 milestone Feb 15, 2024
@jtoar jtoar merged commit e257088 into main Feb 16, 2024
40 checks passed
@jtoar jtoar deleted the ds-deps/remove-types-react-from-testing branch February 16, 2024 00:30
jtoar added a commit that referenced this pull request Feb 16, 2024
…ndencies (#10020)

Have been thinking about this one on and off for a while now and I don't
think any of our packages should have the react types packages as
dependencies if the web workspace in apps is going to have them.

More generally, if a Redwood app is going to explicitly depend on a
package that one of our framework packages also depends on, one of them
should go or use the `'*'` specifier.

I ran into an issue related to this a day or two ago with the deploy
target CI providers. It was easily fixed if you knew what to look for.
Unpinning them was a step forward cause yarn can sometimes make it work
if you run dedupe or at worst edit or regenerate the lock file. But most
people don't know what to look for and we shouldn't expect them to.

The reason we didn't do this before (see the original comment in
#9727) was that the mailer
depends on the react types packages but doesn't explicitly list them as
dependencies. Well, it can still get the react types packages implicitly
from node_modules anyway cause the web workspace puts them there.
dac09 added a commit to dac09/redwood that referenced this pull request Feb 16, 2024
* 'main' of github.com:redwoodjs/redwood: (22 commits)
  fix: Handle static assets on the `rw-serve-fe` (redwoodjs#10018)
  fix(server): fix env var loading in `createServer` (redwoodjs#10021)
  fix(deps): remove react types packages from `@redwoodjs/testing` dependencies (redwoodjs#10020)
  chore(release): add back `update-package-versions` task (redwoodjs#10017)
  chore(renovate): Disable for experimental apollo package (redwoodjs#10016)
  RSC: server cells lowercase data function (redwoodjs#10015)
  fix(RSC/SSR): pass CLI options through to apiServerHandler (redwoodjs#10012)
  7.0 RC: Remove hardcoded check for `session.id` (redwoodjs#10013)
  Spelling fix in what-is-redwood.md (redwoodjs#10011)
  Typos in realtime.md (redwoodjs#10010)
  RSC: Server cell smoke tests (redwoodjs#10008)
  RSC: test-project EmptyUser 'use client' cell (redwoodjs#10007)
  RSC: babel-plugin-redwood-cell remove redundant reset (redwoodjs#10006)
  chore(deps): Upgrade to yarn v4.1.0 (redwoodjs#10002)
  fix(docs): Spelling of `data-migrate` command (redwoodjs#10003)
  docs: add aliases fo `type-check` command (redwoodjs#10004)
  RSC: Insert 'use client' in scaffolded components (redwoodjs#9998)
  fix(telemetry): Fix 'destroy' spelling (redwoodjs#10000)
  chore(jsdocs): Fix jsdoc formatting for hover help (redwoodjs#9999)
  bug: Update setupHandler.ts firebase version (redwoodjs#9997)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:fix This PR is a fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants