Skip to content

fix: import.meta.env.BASE_URL will be '/' in client loaded component on dev mode#4886

Merged
matthewp merged 1 commit intowithastro:mainfrom
yuhang-dong:fix-dev-base-url
Oct 4, 2022
Merged

fix: import.meta.env.BASE_URL will be '/' in client loaded component on dev mode#4886
matthewp merged 1 commit intowithastro:mainfrom
yuhang-dong:fix-dev-base-url

Conversation

@yuhang-dong
Copy link
Copy Markdown
Contributor

Changes

Hi, I have found that import.meta.env.BASE_URL will be set / in client loaded component on dev mode.

And after searching from issues, I found #3567 describe the same situation.

In the conversion, I have found pr#3955 just fix prod mode.

You can visit the below link to see import.meta.env.BASE_URL will be set '/' in client loaded component on dev mode:
https://stackblitz.com/edit/github-vhnc6s-13dn4n?file=src%2Fpages%2Findex.astro
截屏2022-09-27 21 11 56

So, I defined the import.meta.env.BASE_URL to setting.config.base.

Testing

This situation need e2e test, so I added it on [packages/astro/e2e/astro-envs.test.js]

Docs

N/A

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Sep 27, 2022

🦋 Changeset detected

Latest commit: e227d9f

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

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 Sep 27, 2022
include: rendererClientEntries,
},
define: {
'import.meta.env.BASE_URL': settings.config.base ? `'${settings.config.base}'` : 'undefined',
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Would it be the same as the following?

settings.config.base || 'undefined'

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If base is not set, would it not show /undefined/ instead of / in that case here? Since you added undefined in strings?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If base is not set, it would show / in that case here.

What's more, settings.config.base and '${settings.config.base}' are different, notice the single quotes

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@yuhang-dong yup, now I see it. Thanks for the accurate pointers!

Copy link
Copy Markdown

@rishi-raj-jain rishi-raj-jain left a comment

Choose a reason for hiding this comment

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

LGTM

@rishi-raj-jain
Copy link
Copy Markdown

rishi-raj-jain commented Sep 27, 2022

@yuhang-dong

I don't think I'm eligible to post a 'approved'

@matthewp matthewp merged commit 61d26f3 into withastro:main Oct 4, 2022
@astrobot-houston astrobot-houston mentioned this pull request Oct 4, 2022
@bluwy bluwy mentioned this pull request Oct 5, 2022
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.

3 participants