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

A command field in astro:config:setup returns build value while running check command #10876

Open
1 task done
moose96 opened this issue Apr 25, 2024 · 3 comments
Open
1 task done
Labels
- P2: nice to have Not breaking anything but nice to have (priority) feat: integration api related to the public integration APIs (scope)

Comments

@moose96
Copy link
Contributor

moose96 commented Apr 25, 2024

Astro Info

Astro                    v4.7.0
Node                     v18.18.0
System                   Linux (x64)
Package Manager          unknown
Output                   static
Adapter                  none
Integrations             test

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

Let's assume an Astro config like below:

export default defineConfig({
  integrations: [
    {
      name: 'test',
      hooks: {
        'astro:config:setup': ({ command }) => {
          console.log(command);
        },
      },
    },
  ],
});

While running astro check the console outputs build string. This can be confusing, especially when running the command in the default pipeline: astro check & astro build. In this case, the astro:config:setup hook is called twice with command === 'build', unexpectedly triggering certain code twice during the build.
Also, this behavior isn't documented: https://docs.astro.build/en/reference/integrations-reference/#command-option

It appears to be caused by a hardcoded 'build' value in the mentioned line:https://github.com/withastro/astro/blob/main/packages/astro/src/core/sync/index.ts#L62

const settings = await runHookConfigSetup({
	settings: _settings,
	logger: logger,
	command: 'build', // <- here
});

What's the expected result?

I suggest three possible solutions:

  1. command while running astro check has check value
  2. astro check command is integrated with astro build always, so astro:config:setup calls once only (I think it's the worst option)
  3. Describe this behavior in documentation

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-yfhwgr?file=astro.config.mjs

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Apr 25, 2024
@matthewp matthewp added - P2: nice to have Not breaking anything but nice to have (priority) and removed needs triage Issue needs to be triaged labels Apr 25, 2024
@matthewp
Copy link
Contributor

Seems reasonable, would love a PR for this!

@Princesseuh
Copy link
Member

This is probably not astro check's doing, but a side effect of astro check running astro sync implicitly

@moose96
Copy link
Contributor Author

moose96 commented Apr 25, 2024

This is probably not astro check's doing, but a side effect of astro check running astro sync implicitly

You're right. Do you think the astro sync command could hardcode the sync value safely, while passing command property to runHookConfigSetup? Also, I've noticed the sync function has an optional second argument of type SyncOptions. Perhaps enhancing this function to accept properties like runByCommand via SyncOptions could provide better information about executed command? Like when it's called implicitly by astro check, the runByCommand would be check, but when astro sync it's called explicitly, it would be sync by default?

@florian-lefebvre florian-lefebvre added the feat: integration api related to the public integration APIs (scope) label Apr 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P2: nice to have Not breaking anything but nice to have (priority) feat: integration api related to the public integration APIs (scope)
Projects
None yet
Development

No branches or pull requests

4 participants