Skip to content

Conversation

@btea
Copy link
Contributor

@btea btea commented Apr 24, 2024

Description

As shown in the figure below, when the dynamic import component fails, the error message Unknown variable dynamic import: ./components/a/b/test.vue will appear.

But judging from the code structure, there is no problem with the path. I think this information may not be enough for people to immediately and clearly determine where the problem occurs, so outputting pattern information may be able to explain the problem more clearly and help users locate the problem more quickly.

image

@bolt-new-by-stackblitz
Copy link

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@bluwy
Copy link
Member

bluwy commented Apr 29, 2024

I think in general, we want to hide import.meta.glob as an implementation detail of dynamic imports only, so exposing it in the error message could be more confusing?

IIUC the issue is that some may forgot that ${var} cannot contain slashes. Could we detect cases like this in the error message instead? (maybe checking number of /). Or I suppose linking directly to https://vitejs.dev/guide/features.html#dynamic-import could be fine too. We need to also make sure to not make the helper code too large as it's injected to the runtime.

@btea
Copy link
Contributor Author

btea commented Apr 29, 2024

It makes sense, I just noticed that the document states Note that variables only represent file names one level deep.

Maybe we can add the variable inclusion level depth may be more than one, please refer to https://vitejs.dev/guide/features.html#dynamic-import in the error message.

@bluwy
Copy link
Member

bluwy commented Apr 29, 2024

I think maybe directly appending Note that variables only represent file names one level deep should also be fine. It would be nice to optionally show it if we know it happens, but if not it also works for me.

The link currently doesn't have a lot of information, so might not be worth adding it for now.

@btea btea changed the title chore: dynamic import variable failure error message display pattern chore: update dynamic import variable failure error message Apr 30, 2024
Copy link
Member

@bluwy bluwy left a comment

Choose a reason for hiding this comment

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

Seems to work locally for me 👍 Curious to hear what others think of this extra code added before merging it.

@bluwy bluwy changed the title chore: update dynamic import variable failure error message feat: improve dynamic import variable failure error message Apr 30, 2024
@bluwy bluwy added the p2-nice-to-have Not breaking anything but nice to have (priority) label Apr 30, 2024
Copy link
Member

@patak-dev patak-dev left a comment

Choose a reason for hiding this comment

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

I'm good with the added param. Another option is to add a link that is always there and explain more in the docs. Or reword it as a tip but it may confuse more than it helps.

@btea
Copy link
Contributor Author

btea commented Apr 30, 2024

Maybe we can add a detailed explanation chapter about this situation to the troubleshooting page in the future.

@bluwy bluwy merged commit f8feeea into vitejs:main May 2, 2024
@btea btea deleted the feat/improve-dynamic-import-var-message branch May 2, 2024 04:06
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)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants