-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add dev app #16
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@teallarson what's the benefit of a separate app instead of adding an env var to the existing one controlling whether we pull the embedded library from the cdn or locally?
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm 🎸
dev/README.md
Outdated
NEXT_PUBLIC_CLIENT_ID= | ||
NEXT_PUBLIC_CLIENT_SECRET= | ||
NEXT_PUBLIC_WORKSPACE_ID= | ||
NEXT_PUBLIC_BASE_URL= |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add an example or write down that when running locally, this needs to include the /api/public
? It might be implied from the variable name, but I suspect I know because I got burnt before and it's not that obvious
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe name it NEXT_PUBLIC_AIRBYTE_PUBLIC_API_BASE_URL
? Verbose, but that would have saved me a couple times 😄
} catch (widgetError) { | ||
throw widgetError; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unnecessary try/catch here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adds a NextJS dev app to ease local development: