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: Better docs for Vite folks #117

Merged
merged 12 commits into from
May 5, 2023

Conversation

tcc-sejohnson
Copy link
Collaborator

@tcc-sejohnson tcc-sejohnson commented May 4, 2023

Added a snippet explaining how to set up our clients with Vite (this has been the #1 issue filing reason thus far):

@vercel/postgres-kysely
@vercel/postgres
@vercel/kv
@vercel/edge-config

@changeset-bot
Copy link

changeset-bot bot commented May 4, 2023

🦋 Changeset detected

Latest commit: 94fb2eb

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

This PR includes changesets to release 4 packages
Name Type
@vercel/edge-config Patch
@vercel/kv Patch
@vercel/postgres Patch
@vercel/postgres-kysely 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

Copy link
Contributor

@ismaelrumzan ismaelrumzan left a comment

Choose a reason for hiding this comment

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

Thanks for the update
I made some suggestions in the section for edge-config

packages/edge-config/README.md Outdated Show resolved Hide resolved
packages/edge-config/README.md Outdated Show resolved Hide resolved
packages/edge-config/README.md Outdated Show resolved Hide resolved
tcc-sejohnson and others added 3 commits May 4, 2023 15:15
Co-authored-by: Ismael <ismael@vercel.com>
Co-authored-by: Ismael <ismael@vercel.com>
@tcc-sejohnson tcc-sejohnson marked this pull request as ready for review May 4, 2023 22:43
@correttojs
Copy link
Collaborator

Thanks for the update @tcc-sejohnson! I will port these changes to https://github.com/vercel/blob as well (which will be moved here soon).

Would it make sense to update our svelte templates and scripts with this snippet?

export default defineConfig(({ mode }) => {
// This check is important!
if (mode === 'development') {
const env = loadEnv(mode);
Copy link
Collaborator

@correttojs correttojs May 5, 2023

Choose a reason for hiding this comment

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

when testing locally I had to change it to

const env = loadEnv(mode, process.cwd())

I also noticed that loadEnv only loads VITE_ variables

Copy link

@IsaacBreen IsaacBreen May 5, 2023

Choose a reason for hiding this comment

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

This didn't work for me. What did work:

    if (mode === 'development') {
        const env = loadEnv(mode, process.cwd(), '');
        dotenvExpand.expand({parsed: env});
    }

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hang it, I copied the wrong example. One sec...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks @IsaacBreen, didn't see your comment until I had already fixed the README! He's right @correttojs, I forgot you have to specify the third parameter (it's the prefix to load, defaults to VITE_).

Copy link

@IsaacBreen IsaacBreen May 5, 2023

Choose a reason for hiding this comment

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

You're welcome! The {parsed: env} part is important too. Vite's loadEnv returns an object of env vars, but dotenvExpand.expand(config) looks for them under config.parsed .

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

lol, thanks -- it's obviously too late over here for me to be useful :upside-down-cowboy:

@tcc-sejohnson
Copy link
Collaborator Author

Would it make sense to update our svelte templates and scripts with this snippet?

Definitely! It'll make them truly zero-config for people who clone them.

@tcc-sejohnson tcc-sejohnson merged commit c4a8aa5 into main May 5, 2023
7 checks passed
@tcc-sejohnson tcc-sejohnson deleted the elliott/add-vite-notes-to-readme branch May 5, 2023 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants