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: yarn rw setup auth clerk fails in canary #5998

Merged
merged 2 commits into from
Jul 22, 2022
Merged

Conversation

jtoar
Copy link
Contributor

@jtoar jtoar commented Jul 19, 2022

While reviewing #5969, I upgraded a redwood app to canary and tried to setup clerk but it failed with:

~/rwprj> yarn rw setup auth clerk
✔ Overwrite existing /api/src/lib/auth.[jt]s? … yes
rwprj/node_modules/@redwoodjs/cli/dist/commands/setup/auth/auth.js:147
  const customRenderOpen = (0, _reduce.default)(_context6 = authProvider.render || []).call(_context6, (acc, component) => acc + newlineAndIndent + '  ' + `<${component}>`, '');
                                                                                      ^

TypeError: Cannot read properties of undefined (reading 'call')
    at addWebRender (/Users/dom/prjcts/rwprj/rwprj/node_modules/@redwoodjs/cli/dist/commands/setup/auth/auth.js:147:87)
    at addConfigToApp (/Users/dom/prjcts/rwprj/rwprj/node_modules/@redwoodjs/cli/dist/commands/setup/auth/auth.js:253:15)
    at Task.task (/Users/dom/prjcts/rwprj/rwprj/node_modules/@redwoodjs/cli/dist/commands/setup/auth/auth.js:381:9)
    at /Users/dom/prjcts/rwprj/rwprj/node_modules/listr/lib/task.js:167:30
    at processTicksAndRejections (node:internal/process/task_queues:96:5)

I pinpointed it to a change in #4880. This new line:

Turns it out has to be an array of strings cause reduce gets called on it later here:

const customRenderOpen = (authProvider.render || []).reduce(
(acc, component) => acc + newlineAndIndent + ' ' + `<${component}>`,
''
)

Not a big deal right now. But all the more reason to either...

  • add unit tests for setup commands, or
  • use some TS in the cli package

@netlify
Copy link

netlify bot commented Jul 19, 2022

Deploy Preview for redwoodjs-docs canceled.

Name Link
🔨 Latest commit 14913b0
🔍 Latest deploy log https://app.netlify.com/sites/redwoodjs-docs/deploys/62da7cfd1bdf02000992d3ae

@jtoar jtoar changed the title fix: make render an array fix: yarn rw setup auth clerk fails in canary Jul 19, 2022
@jtoar jtoar changed the title fix: yarn rw setup auth clerk fails in canary fix: yarn rw setup auth clerk fails in canary Jul 19, 2022
@jtoar jtoar requested a review from Tobbe July 19, 2022 09:07
@jtoar
Copy link
Contributor Author

jtoar commented Jul 19, 2022

Now that it succeeded, I'm noticing something else maybe. In ClerkAuthProvider, where does navigate come from? Is it supposed to be imported from @redwoodjs/router?

const ClerkAuthProvider = ({ children }) => {
  const frontendApi = process.env.CLERK_FRONTEND_API_URL
  if (!frontendApi) {
    throw new Error('Need to define env variable CLERK_FRONTEND_API_URL')
  }

  return (
    <ClerkProvider frontendApi={frontendApi} navigate={(to) => navigate(to)}> // do we need to import this?
      <ClerkLoaded>
        {children}
      </ClerkLoaded>
    </ClerkProvider>
  )
}

See this fixture in the associated codemods PR for more context:

<ClerkProvider frontendApi={frontendApi} navigate={(to) => navigate(to)}>

@Tobbe
Copy link
Member

Tobbe commented Jul 22, 2022

Not a big deal right now. But all the more reason to either...

  • add unit tests for setup commands, or
  • use some TS in the cli package

I'm all for switching to TS (no big surprise I guess). But both options sounds good. One or the other, or both.

@nx-cloud
Copy link

nx-cloud bot commented Jul 22, 2022

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 14913b0. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 14 targets

Sent with 💌 from NxCloud.

@Tobbe Tobbe enabled auto-merge (squash) July 22, 2022 10:33
@Tobbe
Copy link
Member

Tobbe commented Jul 22, 2022

Now that it succeeded, I'm noticing something else maybe. In ClerkAuthProvider, where does navigate come from? Is it supposed to be imported from @redwoodjs/router?

Yes, you are right. But I think it already is, at least it was for me just now when I tried.

It's also part of the clerk setup script

image

BUT I think I missed that in the codemods. Will double check and address in #5969 if needed

@Tobbe Tobbe merged commit 0cc3d6b into main Jul 22, 2022
@Tobbe Tobbe deleted the ds-fix-setup-clerk branch July 22, 2022 11:03
@redwoodjs-bot redwoodjs-bot bot added this to the next-release milestone Jul 22, 2022
dac09 added a commit to dac09/redwood that referenced this pull request Jul 22, 2022
…ctmode-gen

* 'main' of github.com:redwoodjs/redwood:
  fix(deps): update dependency eslint-plugin-jsx-a11y to v6.6.1 (redwoodjs#6017)
  chore(deps): update dependency @playwright/test to v1.24.0 (redwoodjs#6019)
  chore(deps): update dependency @okta/jwt-verifier to v2.6.0 (redwoodjs#6018)
  fix(storybook): add `args` to auto generate docs (redwoodjs#5979)
  fix: `yarn rw setup auth clerk` fails in canary (redwoodjs#5998)
  Provide a possibility to disable flows in dbAuth completely (redwoodjs#5851)
@jtoar jtoar modified the milestones: next-release, v2.2.0 Jul 22, 2022
@jtoar jtoar modified the milestones: next-release, v3.0.0 Sep 2, 2022
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 topic/cli
Projects
Status: Archived
Development

Successfully merging this pull request may close these issues.

2 participants