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

feat(remix,backend): Repo level config for remix #1470

Merged
merged 4 commits into from
Aug 4, 2023
Merged

Conversation

desiprisg
Copy link
Contributor

@desiprisg desiprisg commented Jul 11, 2023

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

Packages affected

  • @clerk/clerk-js
  • @clerk/clerk-react
  • @clerk/nextjs
  • @clerk/remix
  • @clerk/types
  • @clerk/themes
  • @clerk/localizations
  • @clerk/clerk-expo
  • @clerk/backend
  • @clerk/clerk-sdk-node
  • @clerk/shared
  • @clerk/fastify
  • @clerk/chrome-extension
  • gatsby-plugin-clerk
  • build/tooling/chore

Description

  • npm test runs as expected.
  • npm run build runs as expected.

This PR implements the repo level config for remix. Had to make some additions to the backend package, so it may be worthwhile to add those params to the other backend packages as well.

remix env example

CLERK_SIGN_IN_URL=/sign-in
CLERK_SIGN_UP_URL=/sign-up
CLERK_AFTER_SIGN_IN_URL=/
CLERK_AFTER_SIGN_UP_URL=/

@changeset-bot
Copy link

changeset-bot bot commented Jul 11, 2023

🦋 Changeset detected

Latest commit: 172838b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages
Name Type
@clerk/remix Minor
@clerk/backend Minor
@clerk/fastify Patch
gatsby-plugin-clerk Patch
@clerk/nextjs Patch
@clerk/clerk-sdk-node Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@desiprisg desiprisg changed the title feat(remix,backend): Repo level config for remix feat(remix,backend): Repo level config for remix (WIP) Jul 11, 2023
Copy link

@jit-ci jit-ci bot left a comment

Choose a reason for hiding this comment

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

✅ Great news! Jit hasn't found any security issues in your PR. Good Job! 🏆

@desiprisg desiprisg marked this pull request as ready for review July 12, 2023 22:35
@desiprisg desiprisg changed the title feat(remix,backend): Repo level config for remix (WIP) feat(remix,backend): Repo level config for remix Jul 12, 2023
opts.afterSignUpUrl ||
getEnvVariable('CLERK_AFTER_SIGN_UP_URL') ||
(context?.CLERK_AFTER_SIGN_UP_URL as string) ||
'';
Copy link
Contributor

Choose a reason for hiding this comment

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

☁️ If we had a centralized config object, probably this (and the 3 following added lines) would have been the change to support the Repo level config for remix. Right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We'd also have to include the options in authenticateRequest.

Copy link
Member

Choose a reason for hiding this comment

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

@dimkl That is correct. We need to bring this up again now that we have 2 packages using repo-level config otherwise, this will become unwieldy

Copy link
Member

@panteliselef panteliselef left a comment

Choose a reason for hiding this comment

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

💯

@dimkl dimkl merged commit 30fcdd5 into main Aug 4, 2023
8 checks passed
@dimkl dimkl deleted the remix_repo_level_config branch August 4, 2023 12:37
@clerk-cookie clerk-cookie mentioned this pull request Aug 4, 2023
@clerk-cookie
Copy link
Collaborator

This PR has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@clerk clerk locked as resolved and limited conversation to collaborators Aug 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants