Skip to content

Conversation

tony19
Copy link
Contributor

@tony19 tony19 commented Jun 17, 2021

This enables type inference for:

  1. The options arg of a plugin's install function
  2. The options passed to createApp().use()
/**  * Plugin  */ type PluginAOptionType = { option1?: string; option2: number; option3: boolean; } const PluginA = { install(app: App, ...options: PluginAOptionType[]) { } } const PluginB = { install(app: App) { } } /**  * Main  */ createApp({}) .use(PluginA, { option3: true }) // ❌ option2 (required) missing .use(PluginA, {}) // ❌ option2 and option3 (required) missing .use(PluginA) // ✅ .use(PluginA, { option2: 1, option3: true }) // ✅ .use(PluginA, { option1: 'foo', option2: 1, option3: true }) // ✅ .use(PluginB) // ✅ .use(PluginB, {}) // ❌ unexpected plugin option

demo

@tony19 tony19 force-pushed the feat/types/plugin-options branch from c35db09 to 41ea5df Compare June 17, 2021 05:58
@posva

This comment was marked as outdated.

@tony19

This comment was marked as outdated.

@antfu
Copy link
Member

antfu commented Oct 3, 2022

@tony19 Can you help resolve the conflicts? Thanks

tony19 added 2 commits October 7, 2022 20:53
This enables type inference for: 1. The options arg of a plugin's install function 2. The options passed to createApp().use()
@tony19 tony19 force-pushed the feat/types/plugin-options branch from caf4ac0 to e2dc246 Compare October 8, 2022 01:57
@tony19

This comment was marked as resolved.

Copy link
Member

@posva posva left a comment

Choose a reason for hiding this comment

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

I think we should also handle the case where a plugin options are optional and it can be used in both scenarios: with the option provided and without them

@tony19

This comment was marked as outdated.

tony19 and others added 2 commits October 8, 2022 22:39
Co-authored-by: Eduardo San Martin Morote <posva@users.noreply.github.com>
@antfu antfu requested a review from posva October 10, 2022 04:33
@posva
Copy link
Member

posva commented Oct 10, 2022

You were right! but it also should be stricter 😄
I’m reviewing on the phone so I can’t do much code but I will give it a proper review with help later

Copy link
Member

@posva posva left a comment

Choose a reason for hiding this comment

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

I added stricter tests and refactored the options to make it work at tony19-contrib#1

Co-authored-by: Eduardo San Martin Morote <posva@users.noreply.github.com>
@yyx990803 yyx990803 merged commit c513126 into vuejs:main Nov 14, 2022
chrislone pushed a commit to chrislone/core that referenced this pull request Feb 4, 2023
zhangzhonghe pushed a commit to zhangzhonghe/core that referenced this pull request Apr 12, 2023
github-merge-queue bot pushed a commit to ionic-team/ionic-framework that referenced this pull request Jul 16, 2024
Issue number: Internal --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> In v3.2.46 [vue was updated](vuejs/core#3969) to add support for a generic type argument to the `Plugin` type. In #29637 Ionic's Vue plugin was updated to use the generic, but the`@ionic/vue` project is currently installing v3.2.37. This results in a local type checking error and build error: ``` (!) Plugin typescript: @rollup/plugin-typescript TS2315: Type 'Plugin_2' is not generic. src/ionic-vue.ts: (24:24) 24 export const IonicVue: Plugin<[IonicConfig?]> = { ``` ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - Bumps and pins the dev dependency of `vue` to `3.2.46`. There are other breaking changes that Vue has shipped in minor/patch cycles that prevents from updating to the latest. - Resolves the type checking error locally during build of the`@ionic/vue` package ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change: 1. Describe the impact and migration path for existing applications below. 2. Update the BREAKING.md file with the breaking change. 3. Add "BREAKING CHANGE: [...]" to the commit description when merging. See https://github.com/ionic-team/ionic-framework/blob/main/docs/CONTRIBUTING.md#footer for more information. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> Validation steps: 1. Checkout the branch 2. Build `core/` 3. In `packages/vue`, install latest pinned dependencies with `npm ci` 4. Sync the changes to the `vue` package with `npm run sync` 5. Open `/packages/vue/src/ionic-vue.ts` 6. Observe: No type errors on L24 7. Run `npm run build` 8. Observe: No build errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

5 participants