Fix/typescript-config #248

vejja · 2023-10-17T21:37:36Z

This PR aims at improving the TypeScript configuration of the repo

Types of changes

Bug fix (a non-breaking change which fixes an issue)
New feature (a non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Description

Currently, the TypeScript linter emits several warnings throughout the codebase.

We get warnings that the module "#imports" was not found, forcing us to use @ts-ignore overrides
We need to reimport types from far-fetched places (such as 'h3' or 'nitropack'), where they should be available
We need to import standard Nuxt utility functions (such as defineEventHandler from 'h3') from modules dependencies, i.e. that are not directly defined in our package.json
We sometimes need to cast types to 'unknown' before re-casting them to the right type, to suppress type warnings

Solution: The Nuxt core team has formulated recommendations on how to setup the TS configuration of Nuxt modules (https://nuxt.com/docs/guide/going-further/modules).

This involves
1- setting the root tsconfig file to extend to ./.nuxt instead of ./playground/.nuxt
2- setting autoImports to false in .nuxtrc
3- setting the playground tsconfig file to extend ./.nuxt
The configuration is detailed in the reference template for Nuxt3 modules at https://github.com/nuxt/starter/tree/module

In addition, we also need to provide complementary tsconfig files for the runtime/server and runtime/nitro folders, that extends tsconfig.server.json instead of tsconfig.json.
This is directly copied from the reference template for Nuxt3 apps at https://github.com/nuxt/starter/tree/v3

We further add a specific .nuxtrc file in the playground that allows autoImports in this folder only. This allows us to use classic Nuxt syntax (auto-imports) in the playground. This setup is replicated in fixture files for tests to run without modification.

Finally, the content of package.json is also upgraded to follow the Nuxt recommendations, specifically with regards to the npm run dev:prepare script, and to npm package versions.

Checklist:

My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes (if not applicable, please state why)

No changes are required to the documentation and the tests pass.

In order for the types to be correctly re-generated, it is required to run

npm upgrade
npm run dev:prepare

vejja · 2023-10-17T21:46:33Z

step 1: upgrade TS config

@ts-ignore

Now that the TS config is compliant, we can - remove @ts-ignore overrides - import from "#imports" instead of importing from dependencies

vercel · 2023-10-17T22:03:49Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
nuxt-security	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 28, 2023 8:48am

vejja · 2023-10-17T22:05:03Z

step 2 : clean-up ts-ignore overrides and 'h3' imports in src/runtime

vejja · 2023-10-17T22:13:03Z

step 3 : remove 'h3' imports from playground and fixtures

- no need to import NitroAppPlugin from 'nitropack' - rename 'nitro' variable to 'nitroApp' for clarity

vejja · 2023-10-17T22:21:56Z

step 4 : use standard approach in nitro plugins

vejja · 2023-10-17T22:59:52Z

step 5 : type runtimeConfig via nuxt/schema interface
Allows proper type inference for security options, avoids unecessary type re-casts

vejja · 2023-10-17T23:22:01Z

step 6 : this one improves how the lru-cache storage is mounted
Currently when build we have the following warning:

Build is done with some warnings:
- Inlined implicit external unstorage
- Inlined implicit external destr

Instead of inlining the config and mounting later in middleware, we explicitly mount it upfront via config in the nitro:config hook
This allows to later invoke the standard useStorage utility in the rateLimiter middleware

The warning is now suppressed

Baroshem · 2023-10-18T12:55:04Z

Hey,

I will make sure to merge it for 1.0.0-rc.2 :)

Baroshem · 2023-10-18T14:47:17Z

@vejja

I have included your latest fix for inlined implicit as it was blocking me from release. Thanks for that!

I think after the release of 1.0.0-rc.1 you could just revert the last commit and we will be able to merge the rest of your stuff :)

Baroshem · 2023-10-18T16:48:52Z

Update.

Let's make it ready for the 1.0.0-rc.3. I needed to push a fix to 1.0.0-rc.1 because there was a bug that I didn't catch during testing. It was triggered once released to NPM :(

vejja · 2023-10-19T05:58:18Z

Update.

Let's make it ready for the 1.0.0-rc.3. I needed to push a fix to 1.0.0-rc.1 because there was a bug that I didn't catch during testing. It was triggered once released to NPM :(

Was the bug related to this PR? What is NPM complaining about?

Baroshem · 2023-10-19T07:16:49Z

No no! the bug was releated to Nuxt internal.

Basically, when I used the await import to import the vite plugin it was working perfectly until publishing a package to npm.

After publishing and trying to install the package, it was breaking the app and showing error that the plugin is not a function.

I changed the import from await to normal and it started working correctly. Pretty strange

This reverts commit e751f52.

- conform eslint to standard recommendation - fix "template root requires one element" in secret.vue - also conforms 'npm run dev' script setup

Baroshem

Few suggestions from my side

package-lock.json

package.json

Baroshem · 2023-10-25T08:51:42Z

package.json

+    "@nuxt/eslint-config": "^0.2.0",
+    "@types/node": "^18.18.1",
+    "eslint": "^8.50.0",
+    "nuxt": "^3.7.4",


suggestion: Nuxt recently released 3.8 version. Maybe we could migrate to that?

3.8 requires modification to type imports

See https://github.com/nuxt/nuxt/releases/tag/v3.8.0

Only minor modifications are required in our codebase, I will include these as well

Baroshem · 2023-10-25T08:54:27Z

@vejja could you please also resolve the conflict in 99-cspNonce file? It is probably caused by the other PR you created recently.

vejja · 2023-10-26T06:56:16Z

@vejja could you please also resolve the conflict in 99-cspNonce file? It is probably caused by the other PR you created recently.

will do

…escript-config

Baroshem · 2023-10-27T11:44:51Z

Hey @vejja

Could you please adjust this PR? I have merged your previous one about fixing complaine of CSP and it resulted in conflicts here.

Also, could you try to fix the CI?

Here is the output of Vercel:

Thanks! 💚

vejja · 2023-10-27T12:54:43Z

Sure - ~~Would you mind sharing your Vercel config file so that I can reproduce on my end ?~~

Update: pls ignore, managed to reproduce on Vercel with standard setup

vejja · 2023-10-27T16:24:00Z

Hi @Baroshem
Do you know when the Vercel regression happened in the first time ? (the Load plugin failed: pinceau/volar Error [ERR_REQUIRE_ESM]: require() of ES Module)
I have it on my end on the main branch, and I'm trying to revert to a point where it did not exist

Update: pls ignore, the build was not failing because of this error, but because defineAppConfig was not auto-imported by Nuxt

- add .nuxtrc autoImport directive in docs/ - enables building outside of root directory

vejja · 2023-10-28T09:18:29Z

Quick update:

Merge conflicts resolved
Vercel build now passing

Build-time warnings in the vercel logs are not on our side, but on third-party packages (nuxt-themes/docus).
Unfortunately there is nothing we can do. However, aside from polluting the logs, they do not prevent building successfully so it's only a minor annoyance.

Baroshem · 2023-10-30T10:30:17Z

Thanks @vejja ! I will merge it to the 1.0.0-rc.3 branch and test it there.

feat: upgrade nuxt Typescript config

ca3169a

vejja marked this pull request as ready for review October 17, 2023 21:38

remove unecessary workarounds

2df5a5f

Now that the TS config is compliant, we can - remove @ts-ignore overrides - import from "#imports" instead of importing from dependencies

vercel bot had a problem deploying to Preview October 17, 2023 22:04 Failure

use autoImports in playground and fixtures

a19b3f3

vercel bot had a problem deploying to Preview October 17, 2023 22:12 Failure

use defineNitroPlugin helper in nitro plugins

5e714a7

- no need to import NitroAppPlugin from 'nitropack' - rename 'nitro' variable to 'nitroApp' for clarity

vercel bot had a problem deploying to Preview October 17, 2023 22:21 Failure

provide typing on runtimeConfig

adf39e9

vercel bot had a problem deploying to Preview October 17, 2023 22:58 Failure

fix type for unsupportedPolicies in cspSsg

50c8ffe

vercel bot had a problem deploying to Preview October 17, 2023 23:04 Failure

improve rateLimiter storage mounting

e751f52

vercel bot had a problem deploying to Preview October 17, 2023 23:18 Failure

Baroshem added this to the 1.0.0-rc.2 milestone Oct 18, 2023

Revert "improve rateLimiter storage mounting"

5bf020c

This reverts commit e751f52.

vercel bot had a problem deploying to Preview October 19, 2023 07:36 Failure

vejja changed the base branch from chore/1.0.0-rc.1 to main October 19, 2023 07:37

Merge branch 'main' into fix/typescript-config

883de56

vercel bot had a problem deploying to Preview October 19, 2023 07:40 Failure

eslint update

96d831a

- conform eslint to standard recommendation - fix "template root requires one element" in secret.vue - also conforms 'npm run dev' script setup

vercel bot had a problem deploying to Preview October 19, 2023 08:03 Failure

Baroshem requested changes Oct 25, 2023

View reviewed changes

Baroshem changed the base branch from main to chore/1.0.0-rc.3 October 25, 2023 08:53

update package

f58f992

vercel bot had a problem deploying to Preview October 26, 2023 10:30 Failure

Merge remote-tracking branch 'upstream/chore/1.0.0-rc.3' into fix/typ…

a9e151e

…escript-config

vercel bot had a problem deploying to Preview October 26, 2023 11:02 Failure

vejja requested a review from Baroshem October 26, 2023 11:04

Merge branch 'chore/1.0.0-rc.3' into fix/typescript-config

0fa8ace

vercel bot had a problem deploying to Preview October 28, 2023 08:08 Failure

fix vercel build step

3e596bc

- add .nuxtrc autoImport directive in docs/ - enables building outside of root directory

vercel bot deployed to Preview October 28, 2023 08:48 View deployment

Baroshem approved these changes Oct 30, 2023

View reviewed changes

Baroshem merged commit a667406 into Baroshem:chore/1.0.0-rc.3 Oct 30, 2023
3 checks passed

Baroshem mentioned this pull request Oct 30, 2023

Chore/1.0.0 rc.3 #262

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix/typescript-config #248

Fix/typescript-config #248

vejja commented Oct 17, 2023

vejja commented Oct 17, 2023

vercel bot commented Oct 17, 2023 •

edited

Loading

vejja commented Oct 17, 2023 •

edited

Loading

vejja commented Oct 17, 2023

vejja commented Oct 17, 2023

vejja commented Oct 17, 2023

vejja commented Oct 17, 2023 •

edited

Loading

Baroshem commented Oct 18, 2023

Baroshem commented Oct 18, 2023

Baroshem commented Oct 18, 2023

vejja commented Oct 19, 2023

Baroshem commented Oct 19, 2023

Baroshem left a comment

Baroshem Oct 25, 2023

vejja Oct 26, 2023

vejja Oct 26, 2023 •

edited

Loading

Baroshem commented Oct 25, 2023

vejja commented Oct 26, 2023

Baroshem commented Oct 27, 2023

vejja commented Oct 27, 2023 •

edited

Loading

vejja commented Oct 27, 2023 •

edited

Loading

vejja commented Oct 28, 2023 •

edited

Loading

Baroshem commented Oct 30, 2023

Fix/typescript-config #248

Fix/typescript-config #248

Conversation

vejja commented Oct 17, 2023

Types of changes

Description

Checklist:

vejja commented Oct 17, 2023

vercel bot commented Oct 17, 2023 • edited Loading

vejja commented Oct 17, 2023 • edited Loading

vejja commented Oct 17, 2023

vejja commented Oct 17, 2023

vejja commented Oct 17, 2023

vejja commented Oct 17, 2023 • edited Loading

Baroshem commented Oct 18, 2023

Baroshem commented Oct 18, 2023

Baroshem commented Oct 18, 2023

vejja commented Oct 19, 2023

Baroshem commented Oct 19, 2023

Baroshem left a comment

Choose a reason for hiding this comment

Baroshem Oct 25, 2023

Choose a reason for hiding this comment

vejja Oct 26, 2023

Choose a reason for hiding this comment

vejja Oct 26, 2023 • edited Loading

Choose a reason for hiding this comment

Baroshem commented Oct 25, 2023

vejja commented Oct 26, 2023

Baroshem commented Oct 27, 2023

vejja commented Oct 27, 2023 • edited Loading

vejja commented Oct 27, 2023 • edited Loading

vejja commented Oct 28, 2023 • edited Loading

Baroshem commented Oct 30, 2023

vercel bot commented Oct 17, 2023 •

edited

Loading

vejja commented Oct 17, 2023 •

edited

Loading

vejja commented Oct 17, 2023 •

edited

Loading

vejja Oct 26, 2023 •

edited

Loading

vejja commented Oct 27, 2023 •

edited

Loading

vejja commented Oct 27, 2023 •

edited

Loading

vejja commented Oct 28, 2023 •

edited

Loading