Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

Nuxt3 Bridge upgrade #313

Closed
wants to merge 6 commits into from
Closed

Nuxt3 Bridge upgrade #313

wants to merge 6 commits into from

Conversation

sarayourfriend
Copy link
Contributor

@sarayourfriend sarayourfriend commented Oct 12, 2021

Description

Partial upgrade to Nuxt3 via the bridge: https://v3.nuxtjs.org/getting-started/bridge

Does a whole bunch of stuff to make this possible. I made notes in the code when relevant (like moving env and dev out of nuxt.config.ts).

Main changes:

  1. Upgrade packages (of course)
  2. Convert nuxt.config.js to TypeScript
  3. Configure Jest and Babel to work politely with TypeScript
  4. Add a tsconfig.json that extends the nuxt one (only generated after running nuxi dev) based on the one used in Gutenberg
  5. Replace imports from @nuxtjs/composition-api with #app, the alias for nuxt and vue3 dependencies (this is not really well documented yet anywhere in the Nuxt3/bridge documentation. I had to do some digging to figure this out on my own. There is this issue to document the migration from @nuxtjs/composition-api that I suspect will address that particular problem: Document migration guide from @nuxtjs/composition-api nuxt/bridge#209)

Testing Instructions

Checkout this branch and run rm -rf node_modules && npm install. It's wise to remove the node_modules folder I think to avoid any potential for dependency overlap (shouldn't be an issue on subsequent installs). Then run npm run dev and verify the site loads. It won't work fully beause of useFetch but otherwise it should be okay.

Currently unit tests fail due to: https://github.com/nuxt/framework/discussions/1329

We also need to do some refactoring to update uses of useFetch as it is unavailable in bridge: nuxt/bridge#211

Checklist

  • My pull request has a descriptive title (not a vague title like Update index.md).
  • My pull request targets the default branch of the repository (main) or a parent feature branch.
  • My commit messages follow best practices.
  • My code follows the established code style of the repository.
  • I added or updated tests for the changes I made (if applicable).
  • I added or updated documentation (if applicable).
  • I tried running the project locally and verified that there are no visible errors.

Developer Certificate of Origin

Developer Certificate of Origin
Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

@sarayourfriend sarayourfriend changed the title Attempt upgrade (and fail miserably) Attempt Nuxt 3 "bridge" upgrade (and fail miserably) Oct 12, 2021
package.json Show resolved Hide resolved
@obulat obulat mentioned this pull request Oct 14, 2021
7 tasks
testPathIgnorePatterns: ['/e2e/'],
collectCoverage: true,
collectCoverage: false,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because we had two scripts passing collectCoverage=false in our package.json and only one script that actually cared about coverage, I figured it made more sense to enable it on a script-by-script basis when it is needed instead of disabling it everywhere that it doesn't make sense to have/is annoying to have.

@sarayourfriend sarayourfriend changed the title Attempt Nuxt 3 "bridge" upgrade (and fail miserably) Nuxt3 Bridge upgrade Oct 21, 2021
@sarayourfriend
Copy link
Contributor Author

@obulat I'm looking into refactoring the usage of useFetch and I keep running up against a lack of understanding about how to refactor it out.

Would it be correct that we just need to move the logic inside the callback passed to useFetch to a fetch() handler on the components that call useRelated and then assign the response to a data variable?

@sarayourfriend
Copy link
Contributor Author

sarayourfriend commented Oct 22, 2021

Another complication for us: nuxt/bridge#294

useStore is used by the AudioController and it doesn't look like there's a suitable replacement for it yet (though I'm double checking that now). I'm not totally sure how we could refactor it to useState either unless there's something similar to the React context API for Vue where we could share state across several instances of a component.

Update: After a little more reading, we could use provide and inject to replace the Vuex store behaviors? @dhruvkb do you see any complications with that approach, given you wrote the original implementation using Vuex?

@sarayourfriend
Copy link
Contributor Author

Closing this PR. It was a useful proof of concept and I think we learned a lot from this exercise, but for the time being upgrading to Nuxt 3/Bridge is just not in our best interest. We can re-evaluate the upgrade path once the APIs for Nuxt 3 stabilize.

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

Successfully merging this pull request may close these issues.

3 participants