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

fix(configuration): Update TypeScript configuration #188

Merged
merged 1 commit into from Jun 22, 2023

Conversation

TeddyGavi
Copy link
Contributor

Contributor checklist


Description

Re-open PR #187 after recent changes as per @andrewtavis instructions

- Added vue-tsc and typescript to dev dependencies
- Added typescript option to nuxt.config and compiler options to tsconfig

From activist/frontend I ran

  yarn add --dev vue-tsc typescript
  • A new types file was generated inside .nuxt immediately. I added related typescript code in nuxt.config.ts and tsconfig.json according to the nuxt3-awesome-starter
    • Changes to yarn.lock were automated 😅

NOTE: If you see this Error: ERROR Cannot start nuxt: Cannot find module 'vue-tsc/out/proxy'
restart docker, prune all images, and run docker-compose up --build

Exceptions were thrown where props aren't defined in defineProps of the <script> block for a given component.

Related issue

@netlify
Copy link

netlify bot commented Jun 12, 2023

Deploy Preview for activist-org canceled.

Name Link
🔨 Latest commit 743cb86
🔍 Latest deploy log https://app.netlify.com/sites/activist-org/deploys/649488f8ee0c790008f6800d

@andrewtavis andrewtavis self-requested a review June 17, 2023 11:18
@andrewtavis
Copy link
Member

@TeddyGavi, some checks that I've made on this:

  • First I brought down your version and checked it locally and found that it wasn't running on docker compose up --build
    • I'm thinking that this is why we're getting the failed checks and no deployment preview via a user error in the build
    • This is what it's saying on the Netlify end: Build failed due to a user error: Build script returned non-zero exit code: 2
  • I checked a few things from the issue that I was getting in the local Docker run: ERROR Cannot start nuxt: Cannot find module 'vue-tsc/out/proxy'
    • Nothing seemed to be particularly insightful for the few Nuxt repo issues I found when I searched for this issue
    • One issue said to pin vue-tsc and another said to reinstall it
  • In following the second suggestion, decided to revert back to my main and retrace your steps
    • Switched to the activist/frontend directory
    • yarn add typescript --dev
    • yarn add vue-tsc --dev
    • Added the lines to nuxt.config.ts and checked the build (was working)
    • Added the lines to tsconfig.json and checked the build (was also working)
  • This is a bit confusing as I was trying to reproduce the Cannot find module 'vue-tsc/out/proxy
  • In checking my package.json, I have "typescript": "^5.1.3" as you do, but when I installed vue-tsc it gave me "vue-tsc": "^1.8.0"
    • As it's building locally on my end and we haven't changed anything, then us having version errors might be what's causing the problems on your end?
    • Maybe in installing vue-tsc first in your command above it installed a lower version that has some dependency mismatches?

@andrewtavis
Copy link
Member

Do you want to try the above steps and send another commit assuming you get "vue-tsc": "^1.8.0"? We'll then check to see if it builds and then we can go from there? 😊

@TeddyGavi
Copy link
Contributor Author

Hey @andrewtavis Thank you for looking into this!

I checked a few things from the issue that I was getting in the local Docker run: ERROR Cannot start nuxt: Cannot find module 'vue-tsc/out/proxy'

This is the exact error I got locally, when I pruned Docker I could build locally, although TS errors appeared in the nuxt output. This is very strange as it doesn't seem like the proxy error is reproducible.

Do you want to try the above steps and send another commit assuming you get "vue-tsc": "^1.8.0"? We'll then check to see if it builds and then we can go from there?

I will try updating the version and see what happens! 👍 😄 .

@andrewtavis
Copy link
Member

Looking forward, @TeddyGavi! Hopefully it works out and then we can close #148 and make some new issues to fix other TS errors 😊

@andrewtavis
Copy link
Member

Really confusing, @TeddyGavi... Did it build locally for you?

@andrewtavis
Copy link
Member

Thanks for keeping at this issue! Super important that we get this working :)

@TeddyGavi
Copy link
Contributor Author

@andrewtavis, yes it did! I just realized, I didn't update the lock file before sending this commit, sending another one asap and rewriting my other commits, lets see what happens. 👍

@TeddyGavi
Copy link
Contributor Author

@andrewtavis The build appears to be failing with "vue-tsc": "^1.8.0" as well. I will have to spend more time reading and looking into this issue. Hopefully, we can get this sorted out soon. I would like to get the TS errors solved 😟

@andrewtavis
Copy link
Member

I just realized, I didn't update the lock file before sending this commit, sending another one asap

The build appears to be failing with "vue-tsc": "^1.8.0" as well.

Did it run on the combined commit? It looks like the build is being triggered by commits and not on the force pushes based on the icons. See that for the below there's the x for the build failing:

Screenshot_2023-06-20 01 16 20

And for here it's missing:

Screenshot_2023-06-20 01 16 41

Not sure, but I guess it's worth a shot to send it all again or just do a commit with a comment in the readme that I could then delete 😅

Copy link
Member

@andrewtavis andrewtavis left a comment

Choose a reason for hiding this comment

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

All seems to be working, @TeddyGavi! 🙌 Really got us through a marathon here 😊 Happy to have this done, and was fun brainstorming with you!

Will write in the issue about whether we need to document this a bit :)

@andrewtavis andrewtavis merged commit ec0eecd into activist-org:main Jun 22, 2023
5 checks passed
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.

None yet

2 participants