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 using Vite env var names in Markdown (#3412) #3471

Merged
merged 1 commit into from
May 30, 2022

Conversation

hippotastic
Copy link
Contributor

Changes

Testing

I added a new test case and ran all tests successfully.

Docs

Not a visible change, just a bugfix.

@changeset-bot
Copy link

changeset-bot bot commented May 29, 2022

🦋 Changeset detected

Latest commit: 2312492

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

This PR includes changesets to release 1 package
Name Type
astro 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

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label May 29, 2022
@aFuzzyBear
Copy link
Contributor

Fantastic 🦛 !!!
This looks amazing, thanks again for the effort on putting this together and really tackling the issue at the root.
💚💚💚

Copy link
Contributor

@aFuzzyBear aFuzzyBear left a comment

Choose a reason for hiding this comment

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

😎 Looking Dam Good 🦛

Copy link
Contributor

@bholmesdev bholmesdev left a comment

Choose a reason for hiding this comment

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

Nice change that should make our docs wayyyy simpler! Just curious though, are you still able to use import.meta.* with explicit JSX expressions? Ex. If I wanted to insert import.meta.env.SITE, could I use {import.meta.env.SITE}? (my hunch is yes it works but just confirming)

@hippotastic
Copy link
Contributor Author

What do you mean with "still able to use", @bholmesdev? This didn't work in Markdown before my changes, and it still doesn't! 😅 I can have a look and try to fix that, too. I agree it would be nice to actually be able to use those in Markdown, too.

@ghost
Copy link

ghost commented May 30, 2022

@hippotastic oh SHOOT I assumed it did work at some point! Well dang that’s a bummer. Still happy to approve this change 👍

Copy link
Contributor

@bholmesdev bholmesdev left a comment

Choose a reason for hiding this comment

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

Very important change 👏

@hippotastic
Copy link
Contributor Author

@hippotastic oh SHOOT I assumed it did work at some point! Well dang that’s a bummer. Still happy to approve this change 👍

Haha yeah, I also assumed that it works and was surprised to see that it didn't when I created this PR. 😅 The {import.meta.env.SITE} JSX syntax was invalid before my changes and it unfortunately still is. I've just had a look and think it will take longer to make this syntax work.

I'd therefore suggest to wrap this PR up first and separate making the JSX syntax work into a new PR.

@bholmesdev bholmesdev merged commit 75fa58f into withastro:main May 30, 2022
@github-actions github-actions bot mentioned this pull request May 30, 2022
@hippotastic hippotastic deleted the fix/vite-env-vars-in-md branch May 30, 2022 16:52
SiriousHunter pushed a commit to SiriousHunter/astro that referenced this pull request Feb 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants