Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…o FW-7656 # Conflicts: # packages/vue-router/tsconfig.json # packages/vue/src/components/IonTabBar.ts # packages/vue/tsconfig.json
thetaPC
approved these changes
Aug 5, 2026
| working-directory: ./packages/vue-router | ||
| # The rollup build reports type errors as warnings and still succeeds, so | ||
| # this step is what keeps the package type-clean. | ||
| - name: 🔎 Typecheck |
Contributor
There was a problem hiding this comment.
Is it worth adding to the other frameworks? If not, then adding a comment here would be great to remind us of why only these packages.
|
|
||
| * Tests are found in the `__tests__` directory and use Jest. | ||
| * Tests can be run using `npm run test.spec` | ||
| * This package has no unit tests of its own. Its behavior is covered by the `@ionic/vue` test app, in `packages/vue/test/base/tests`. See [Vue Testing](../vue/testing.md). |
Contributor
There was a problem hiding this comment.
Suggested change
| * This package has no unit tests of its own. Its behavior is covered by the `@ionic/vue` test app, in `packages/vue/test/base/tests`. See [Vue Testing](../vue/testing.md). | |
| * This package has no unit tests of its own. Its behavior is covered by the `@ionic/vue` test app, in `packages/vue/test/base/tests`. Refer to [Vue Testing](../vue/testing.md). |
2 tasks
ShaneK
added a commit
that referenced
this pull request
Aug 6, 2026
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? Currently, `@ionic/react` and `@ionic/react-router` build with rollup and `@rollup/plugin-typescript`, which reports type errors as `(!) [plugin typescript]` warnings and still exits 0. Neither `build-react` nor `build-react-router` runs `tsc --noEmit`, so a type error can't fail CI. This is the same gap #31320 closed for `@ionic/vue` and `@ionic/vue-router`, and it came out of review feedback on that PR asking whether the typecheck step was worth adding to the other frameworks. ## What is the new behavior? Both React packages now have a `typecheck` script and a CI step that runs it between lint and build, matching the Vue packages. Neither package needed source changes: both already pass `tsc --noEmit` clean under the `strict` they inherit from `core/tsconfig.base.json`, so this is a guard against future regressions rather than a cleanup. Angular is deliberately left alone. `ng-packagr` fails the build on type errors, so a separate step there would be redundant. I added a comment to `build-angular` and `build-angular-server` saying so, so the next person doesn't have to work it out again. ## Does this introduce a breaking change? - [ ] Yes - [X] No ## Other information Follow-up to [#31320](#31320), addressing @thetaPC's review comments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue number: internal
What is the current behavior?
Currently,
packages/vueandpackages/vue-routerinheritstrictfromcore/tsconfig.base.jsonas of #31280, but the 61 errors that turns on were never fixed: 41 in@ionic/vue(24 of them inIonTabBar) and 20 in@ionic/vue-router. Neither package setsrootDir.Nothing reports them today. The rollup build prints type errors as
(!) [plugin typescript]warnings and still exits 0, and there's notsc --noEmitstep in either CI action, so a type error cannot fail a build.Two of the errors are actual bugs, which I went ahead and fixed here.
What is the new behavior?
Both packages now typecheck clean, and
rootDir: "src"is set to match the other packages. Most of the 61 fixes are null guards and signatures that were lying about what they returned.getTabsHistorywas declared(tab: string) => RouteInfo[]but everything using this was already treating it asstring | undefinedanyway.tabs: {}inIonTabBar's data inferred{}rather than theTabStateinterface already declared in the same file._tabsWillChangeand_tabsDidChangewere initialized to a Vue prop-descriptor object instead of a function, which is why calling them wasn't type-safe.Changing
rootDirdidn't do actually anything here, it just makes us consistent.@rollup/plugin-typescriptalready inferssrcas the common root, and I verifieddist/for both packages with and without it to confirm nothing changed.Both bugs above are fixed.
One behavior change worth calling out: Pressing back with no
defaultHrefand nothing in history used to callrouter.replace(undefined), which vue-router resolves against the current location and then rejects asNAVIGATION_DUPLICATED, so it already did nothing. That path now returns without navigating, matching@ionic/react-router, and it also clears the pop/backincomingRouteParamsthat the old code left staged for the next navigation to pick up.ion-back-buttonalso reads thebackButtonDefaultHrefconfig now. Core resolves that config itself and only renders the button once it has a href, so a config-only back button was previously visible but did nothing.CI now runs a
typecheckscript in bothbuild-vueandbuild-vue-router, between lint and build, preventing us from having to do this cleanup again in the future.Does this introduce a breaking change?
Other information