Skip to content

feat(vue)!: separate include and exclude from api.options and add filter #582

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

Merged
merged 5 commits into from
May 21, 2025

Conversation

sapphi-red
Copy link
Member

Description

Adds filter to plugin-vue so that it makes the plugin performant when used in rolldown-vite.

Additional context


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the PR Title Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@sapphi-red sapphi-red added the p3-significant 🔨 High priority enhancement (priority) label Apr 28, 2025
Copy link

pkg-pr-new bot commented Apr 28, 2025

Open in StackBlitz

npm i https://pkg.pr.new/@vitejs/plugin-vue@582
npm i https://pkg.pr.new/@vitejs/plugin-vue-jsx@582

commit: 962d845

Comment on lines 404 to 410
filter: {
// FIXME: should include other files if filter is updated
id: {
include: /[?&]vue\b|\.vue$/,
exclude: /[?&](?:raw|url)\b/,
},
},
Copy link
Member Author

Choose a reason for hiding this comment

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

To have this part of change, we need to introduce a breaking change.
The filter has to have a same value after configResolved hook is called (because the value is passed to rolldown and cannot be modified), but currently options.value.include / options.value.exclude can be changed in any time.

I can revert this part for now so we can merge the other parts and make a separate PR if that's better.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it doesn't apply to those cases.
In this plugin-vue's case, options.value is exposed via api, which is meant to be used to update the option.value. So I think the contract here is that setting api.options will update the option.
On the other hand, in that unplugin-auto-import case, I think people won't expect options.include to be modified after the plugin is instantiated. For example, the code below would do that, but I guess people won't expect this to work unless explicitly documented.

const options = { include: [] }
setTimeout(() => {
  options.include.push('something') // or options.include = ['something-else']
}, 1000)
const plugin = autoImport(options)

Copy link
Member Author

Choose a reason for hiding this comment

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

@sxzz What are your thoughts on this? (since you introduced this feature)

Copy link
Member

Choose a reason for hiding this comment

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

Vue Macros is using this API to modify some options like template.compilerOptions.

Can we make api.include/exclude as a getter/setter, linking to transform.filter?

Copy link
Member Author

Choose a reason for hiding this comment

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

Can we make api.include/exclude as a getter/setter, linking to transform.filter?

Do you mean something like below?

let filter = {}
const p = {
  api: { get include() { return filter.include }, set include(v){ filter.include = v } },
  transform: { filter, handler() { /* omit */ } }
}

Is it to make it clear that include/exclude cannot be updated after configResolved hook is called?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, we can add a jsdoc

Copy link
Member Author

Choose a reason for hiding this comment

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

@sxzz I've updated the PR 👍

@sapphi-red sapphi-red marked this pull request as ready for review May 20, 2025 06:33
@sapphi-red sapphi-red changed the title feat(vue): add filter feat(vue)!: add filter May 20, 2025
@sapphi-red sapphi-red changed the title feat(vue)!: add filter feat(vue)!: separate include and exclude from api.options and add filter May 20, 2025
@sapphi-red
Copy link
Member Author

@sxzz @haoqunjiang @edison1105 Do you have any thoughts on the next major release?

@edison1105
Copy link
Member

LGTM~

@sxzz
Copy link
Member

sxzz commented May 20, 2025

I prefer to release a beta major version.

@sapphi-red
Copy link
Member Author

OK. I'll merge this and cut a beta.

@sapphi-red sapphi-red merged commit e3beac8 into main May 21, 2025
18 checks passed
@sapphi-red sapphi-red deleted the feat/vue-add-filter branch May 21, 2025 05:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p3-significant 🔨 High priority enhancement (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants