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!(nuxt): upgrade nuxt module #1435

Merged
merged 30 commits into from
Jul 13, 2022
Merged

feat!(nuxt): upgrade nuxt module #1435

merged 30 commits into from
Jul 13, 2022

Conversation

pi0
Copy link
Contributor

@pi0 pi0 commented Jul 8, 2022

Rework of #791

  • Add pinna as a dependency -- Nuxt modules are supposed to be standalone and provide integration out of the box
  • Add simple playground
  • Migrate to module-builder and nuxt kit
  • Split vue 2 (bridge) and vue 3 plugins
  • Remove deprecated ctx.pinia
  • Nuxt 2 support requires bridge to be enabled (using plain simple plugin)
  • Use nuxi typecheck for tpyechecking module
  • vue-demi is removed for vue2 support since bridge is in place
  • Auto imported composables (definePiniaStore() and usePinia() exposed)

Breaking changes:

  • Nuxt 2 users need to enable nuxt bridge in order to use latest version of the module.
  • Only $pinia injection is available which can be accessed using usePinia() (or useNuxtApp().$pinia)
  • $nuxt from store can be accessed using useNuxtApp() composable

To be tested:

  • Nuxt 3 on an external project
  • Nuxt 2 (w, w/o bridge) on an external project

@netlify
Copy link

netlify bot commented Jul 8, 2022

Deploy Preview for pinia-official canceled.

Name Link
🔨 Latest commit 5286aee
🔍 Latest deploy log https://app.netlify.com/sites/pinia-official/deploys/62ce9ee67bd2d80008dcd16d

@codecov-commenter
Copy link

codecov-commenter commented Jul 8, 2022

Codecov Report

Merging #1435 (e12ad3a) into v2 (e2e9f1d) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##               v2    nuxt/nuxt.js#12596   +/-   ##
=======================================
  Coverage   97.59%   97.59%           
=======================================
  Files           9        9           
  Lines         416      416           
  Branches      109      109           
=======================================
  Hits          406      406           
  Misses          5        5           
  Partials        5        5           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e2e9f1d...e12ad3a. Read the comment docs.

@pi0 pi0 changed the title feat: upgrade nuxt module feat!: upgrade nuxt module Jul 8, 2022
@pi0 pi0 marked this pull request as ready for review July 8, 2022 12:00
@pi0 pi0 changed the title feat!: upgrade nuxt module feat!(nuxt): upgrade nuxt module Jul 8, 2022
@pi0 pi0 mentioned this pull request Jul 8, 2022
5 tasks
packages/nuxt/src/module.ts Outdated Show resolved Hide resolved
@posva
Copy link
Member

posva commented Jul 8, 2022

I force pushed because due to conflicts in pnpm-lock

@posva
Copy link
Member

posva commented Jul 8, 2022

The types never worked on my end. It's as if vue-tsc didn't pick up the correct tsconfig so I removed them from CI

@posva
Copy link
Member

posva commented Jul 8, 2022

Do you also get this one:

Screenshot 2022-07-08 at 17 38 27

It didn't appear before...

@posva
Copy link
Member

posva commented Jul 8, 2022

So I tested and this only works with Nuxt 3 😓
It fails with Nuxt 2 and Nuxt bridge with different errors

  • In Nuxt 2 it seems it's importing two different versions of Pinia but I haven't been able to debug further
  • In Nuxt Bridge, it's yielding
 WARN  Disabling duplicate auto import 'undefined' (imported from '/Users/posva/piniajs/example-nuxt-bridge/node_modules/@pinia/nuxt/dist/runtime/composables')

and

Cannot read properties of undefined (reading 'install')
  at Function.Vue.use (file://./.nuxt/dist/server/server.mjs:7840:23)
  at $id_6aebe68d (file://./.nuxt/dist/server/server.mjs:22659:5)
  at async __instantiateModule__ (file://./.nuxt/dist/server/server.mjs:22861:3)

@pi0
Copy link
Contributor Author

pi0 commented Jul 11, 2022

Thanks for the review @posva. I will rebase and check last changes on bridge.

pi0 and others added 21 commits July 13, 2022 12:27
BREAKING CHANGE: in Nuxt 3, `$nuxt` is no longer available in stores.
This is because it was removed in Nuxt 3 and it is no longer the
_context_ as it used to be. Most of the features used there, like
`$fetch` are now globally available and therefore remove the need of it.
You can also use
[`useNuxtApp()`](https://v3.nuxtjs.org/bridge/bridge-composition-api/)
when necessary.
BREAKING CHANGE: Starting on this version, `@pinia/nuxt` only works with
Nuxt 2 + Bridge and Nuxt 3, it no longer works with Nuxt 2 only. This is
necessary to have one single plugin that works well with the different
versions of Nuxt. If you aren't using bridge with Nuxt 2, check out the
[migration guide](https://v3.nuxtjs.org/bridge/overview) or pin your
`@pinia/nuxt` dependency in your:

```diff
-    "@pinia/nuxt": "^0.2.1",
+    "@pinia/nuxt": "0.2.1",
```

The `$nuxt` context usage should be replaced with globals like
`$fetch()` and `useNuxtApp()`. You can find more information about this
in Nuxt documentation:

- https://v3.nuxtjs.org/bridge/bridge-composition-api/
- https://v3.nuxtjs.org/bridge/overview
BREAKING CHANGE: `$nuxt` usage in stores defaults to type `any` unless
you install the `@nuxt/types` package. This is because that package is
quite heavy and can cause conflicts in projects not requiring it. Note
`$nuxt` is deprecated and shouldn't be used (cf the other breaking
changes notes).
@posva posva linked an issue Jul 13, 2022 that may be closed by this pull request
@posva posva merged commit 5c79f14 into vuejs:v2 Jul 13, 2022
@pi0 pi0 deleted the feat/upgrade-nuxt branch July 13, 2022 12:36
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.

Nuxt3 - Cannot set properties of undefined (setting 'pinia')
4 participants