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

feat: allow components to provide absolute import path #234

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

antfu
Copy link
Member

@antfu antfu commented Sep 15, 2021

This would allow libraries to provide absolute import paths (from library submodules, or virtual modules) using the components:extend hook. In my case, I am integrating with unplugin-icons.

@pi0
Copy link
Member

pi0 commented Sep 15, 2021

I think it should still work by setting a relative path to node_modules. Are you using a virtual import pattern? An example would be nice :)

@@ -4,13 +4,14 @@
c.prefetch === true || typeof c.prefetch === 'number' ? `webpackPrefetch: ${c.prefetch}` : false,
c.preload === true || typeof c.preload === 'number' ? `webpackPreload: ${c.preload}` : false,
].filter(Boolean).join(', ')
const filePath = c.isAbsolute ? c.filePath : `../${relativeToBuild(c.filePath)}`
Copy link
Member

Choose a reason for hiding this comment

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

We would need to normalize windows paths in fullPath condition

@antfu
Copy link
Member Author

antfu commented Sep 15, 2021

The import for unplugin-icons uses something like import Icon from '~icons/logos/nuxt-icon', which in the original behavior it will always prepend with ../ and got this error:

image

See unplugin/unplugin-icons#63 for the implementation.

This other case I can think about might be importing from some libraries, like: import Button from 'element-ui/lib/button'

As for the windows path resolution, I guess since the isAbsolute can only be provided by hooks, we can leave the responsibility to the module author?

@pi0
Copy link
Member

pi0 commented Sep 15, 2021

I'm mostly hesitating to see if we really need another component option or can actually fix the situation by default. For resolving ~, for instance we can check prefix to disable resolution on virtual and ~, @.

This other case I can think about might be importing from some libraries, like: import Button from 'element-ui/lib/button'

This case makes sense. But maybe with a flag like resolvePath: false ?

One more point, we usually allow inherit options from dirOptions (apply option to a group of scanned components) (unplugin/unplugin-icons#63 (comment))

@antfu
Copy link
Member Author

antfu commented Sep 15, 2021

Do you mean we rename isAbsolute to resolvePath?

@pi0
Copy link
Member

pi0 commented Sep 15, 2021

Yes please, and also auto-enable the flag when ~, @ prefixes was provided if possible :)

@atinux atinux requested a review from pi0 September 29, 2021 16:30
@jd-solanki
Copy link

Sorry guys I don't want to disturb you here but I guess this is stale for a long time. Will this get updated?

Actually, I am waiting for this PR unplugin/unplugin-icons#63

Regards.

@AqueleHaru
Copy link

Still waiting too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants