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(serverless): move project-config to auth-dbauth-api dependencies #9281

Merged
merged 1 commit into from
Oct 8, 2023

Conversation

jtoar
Copy link
Contributor

@jtoar jtoar commented Oct 8, 2023

After cherry picking #9248, Netlify and Vercel deploys were broken with the following error:

image

It took me a second to realize it, but @redwoodjs/project-config is listed as a dev dependency in @redwoodjs/auth-dbauth-api despite being used in a source file:

import { getConfig } from '@redwoodjs/project-config'

@Tobbe this may not be the end of it—in those environments, there's no redwood.toml either, so I think getConfig will throw. But we can solve one bug at a time.

@jtoar jtoar added the release:fix This PR is a fix label Oct 8, 2023
@jtoar jtoar added this to the next-release milestone Oct 8, 2023
@jtoar jtoar requested a review from Tobbe October 8, 2023 12:58
@jtoar jtoar enabled auto-merge (squash) October 8, 2023 21:29
@jtoar jtoar disabled auto-merge October 8, 2023 21:29
@jtoar jtoar merged commit 229bd4b into main Oct 8, 2023
31 of 32 checks passed
@jtoar jtoar deleted the ds-serverless/mv-devDep-to-dep branch October 8, 2023 21:30
jtoar added a commit that referenced this pull request Oct 8, 2023
…9281)

After cherry picking #9248,
Netlify and Vercel deploys were broken with the following error:

<img width="1532" alt="image"
src="https://github.com/redwoodjs/redwood/assets/32992335/2e366cd9-48e7-46e6-a32e-1c7872418674">

It took me a second to realize it, but `@redwoodjs/project-config` is
listed as a dev dependency in `@redwoodjs/auth-dbauth-api` despite being
used in a source file:

https://github.com/redwoodjs/redwood/blob/040589a4e600e298837e3f2b7373d1c2403eb784/packages/auth-providers/dbAuth/api/src/shared.ts#L4

@Tobbe this may not be the end of it—in those environments, there's no
`redwood.toml` either, so I think `getConfig` will throw. But we can
solve one bug at a time.
@jtoar jtoar modified the milestones: next-release, v6.4.0 Oct 10, 2023
jtoar added a commit that referenced this pull request Nov 3, 2023
Follow up to #9281. The
functionality added in #9248
depends on the `redwood.toml` file. This file doesn't exist in
serverless environments.

A quick note on the implementation. I could've done this:

```js
let config

try {
  config = getConfig()
} catch (e) {
  if (e.message !== `Could not find a "redwood.toml" file, are you sure you're in a Redwood project?`) {
    throw e
  }
}

// ...
```

But that seemed more brittle because it depends on the error message.

Lastly, `getConfig` merges defaults with the raw config, so I don't
think the `api?.port` is necessary.
jtoar added a commit that referenced this pull request Nov 3, 2023
Follow up to #9281. The
functionality added in #9248
depends on the `redwood.toml` file. This file doesn't exist in
serverless environments.

A quick note on the implementation. I could've done this:

```js
let config

try {
  config = getConfig()
} catch (e) {
  if (e.message !== `Could not find a "redwood.toml" file, are you sure you're in a Redwood project?`) {
    throw e
  }
}

// ...
```

But that seemed more brittle because it depends on the error message.

Lastly, `getConfig` merges defaults with the raw config, so I don't
think the `api?.port` is necessary.
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