Improve Vue appEntrypoint handling

[yoyo837]

Changes

Improve robustness and compatibility for some case.

When vue's appEntrypoint is configured and it does not return the default function, it will throw an error:

astro.config.mjs:

  integrations: [
    vue({
      appEntrypoint: '/src/pages/_app',
      template: {
        compilerOptions: {
          //
        },
      },
    }),
  ],

src/app.js:

// Comment it out, so no export default function
// export default async app => {
//   await global(app);
// };

An error will be thrown after running:

Testing

Check if a function is available before calling it.

Docs

[changeset-bot]

🦋 Changeset detected

Latest commit: 2500fdc

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

[natemoo-re]

Code looks fine, would like to know why we need this though!

[natemoo-re]

This is pretty non-descriptive and I don't have any info to go off of in the PR description. Would you mind describing specifically why you made these changes?

[yoyo837]

Yes.

astro.config.mjs:

  integrations: [
    vue({
      appEntrypoint: '/src/pages/_app',
      template: {
        compilerOptions: {
          //
        },
      },
    }),
  ],

In the /src/pages/_app.js file, if there is no export defualt (app) => {}, it may cause errors.

[ematipico]

Blocking, for the following reasons:

• the template is empty and doesn't explain anything about the PR
• we don't know what we are fixing, there isn't a test
• there's no test nor explanation about this fix being tested

[yoyo837]

Blocking, for the following reasons:

• the template is empty and doesn't explain anything about the PR
• we don't know what we are fixing, there isn't a test
• there's no test nor explanation about this fix being tested

There is my first astro PR. Is there any documentation on how to create a standard PR?

[ematipico]

Blocking, for the following reasons:

• the template is empty and doesn't explain anything about the PR
• we don't know what we are fixing, there isn't a test
• there's no test nor explanation about this fix being tested

There is my first astro PR. Is there any documentation on how to create a standard PR?

Edit the description of the PR. Under Changes explain what this change is about, you gave a great explanation to Nate before, you can do the same in the PR. Explain the bug and how you fixed that.

Under the Testing explain how you tested your fix. Ideally, you should create a new test inside the integration.

No need for documentation, so you can leave that part empty.

[yoyo837]

Thanks. I'll try it later.

[ematipico]

Hi @yoyo837 , are you still interested in this PR?

[yoyo837]

Hi @yoyo837 , are you still interested in this PR?

Yes, I still want to finish this PR, but I also wouldn't mind opening a new PR when I actually do it.

[yoyo837]

When I add the test case, I got some error:

RollupError: "default" is not exported by "test/fixtures/app-entrypoint-no-export-default/src/pages/_app.ts", imported by "virtual:@astrojs/vue/app".

it seems that Rollup does not allow no export default declaration.

https://rollupjs.org/troubleshooting/#error-name-is-not-exported-by-module

[ematipico]

I think the error is correct. Maybe what you want to do is to use import * as virtualApp instead

[yoyo837]

I think the error is correct. Maybe what you want to do is to use import * as virtualApp instead

Need help.

[ematipico]

I am not very familiar with the integration, but that is failing because in case of appEntrypoint, we don't export a default export

astro/packages/integrations/vue/src/index.ts

Lines 46 to 50 in 35cd810

	if (id === resolvedVirtualModuleId) {
	if (options?.appEntrypoint) {
	return `export { default as setup } from "${options.appEntrypoint}";`;
	}
	return `export const setup = () => {};`;

[natemoo-re]

Thanks for picking this up again @florian-lefebvre! Just pushed some changes but this LGTM.

[yoyo837]

Thank you everyone for making this PR landed.