Feat [breaking changes]: async loading

[flozero]

Resolve #19 #20 #18

Types of changes

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

Description

• Create a singleton for the instance of stripe client side and make it lazy loaded by default so it wont be loaded until you use it at least once
• update config for stripe in nuxt config to be a bit clearer
• now let the user add options on client and server side stripe
• update documentation

Checklist:

• [x] My change requires a change to the documentation.
• [x] I have updated the documentation accordingly.
• [ ] I have added tests to cover my changes (if not applicable, please state why). The nuxt test integration is pretty bad tried to add some test but the doc is inexistant for now

[sandros94]

I'm sorry, I don't fully understand why open up another PR when there is already an existing one with an ongoing discussion with the other maintainers of the project...

This said I do like the approach of exposing other client/server configs directly to the nuxt.config but relying on the addPlugin it will still make the stripe-js instance load all across the nuxt project instead of where the developers wants it to, degrading the performance of the website/app after the user has visited the stripe-js checkout. I know there isn't proper testing for this, but IMO performance should be taken in consideration.

I'm on mobile rn, so I cannot do some more testings, but why wrap around the useClientStripe in a ClientOnly if it is already something dealt with?

[sandros94]

A short list of inconsistencies in this PR I've noticed so far:

• missing server side vitests.
• missing docs for runtime configs via env variables.
• missing docs for server/client side options.
• delayed stripe-js load and not truely lazyloading it.
• not exposing types for stripe-js (could be an issue on my side tho, I need someone else to confirm this).
• repetitive checks for stripe-js being loaded server-side (loadStripe already checks and report this).
• keys initialized as nulls instead of empty strings (I'm not an expert yet, but this could lead to types inconsistencies or at least its not a good practice, already mentioned in the original PR).

I would like to hear your opinion @flozero, in particular why open up a new PR that doesn't fully solve the already discussed topics and where did you find the docs for the additional configuration options for both client and server side stripe modules?

[sandros94]

I would like to point out another small inconsistency that could degrade the DX for less experienced developers.

Through both stripe and stripe-js docs (and in their respective typescript interfaces) it is mentioned multiple times the structure of stripe(apiKey, config) and stripe(publishableKey, options).

IMHO I would keep this structure for a better DX and docs fruition, as I have already done in #19:

export default defineNuxtConfig({
  modules: [
    '@unlok-co/nuxt-stripe'
  ],
  stripe: {
    // Server
    apiKey: 'sk_test_123', // required
-   apiVersion: '2022-11-15', // optional, default is '2022-11-15'
+   serverConfig: {
+    apiVersion: '2022-11-15', // optional, default is '2022-11-15'
+   },
    // Client
    publishableKey: 'pk_test_123', // required
+   clientConfig: {
+    apiVersion: '2022-11-15', // optional, default is '2022-11-15'
+   },
  }
})

and not to use the proposed change of this PR:

export default defineNuxtConfig({
  modules: [
    '@unlok-co/nuxt-stripe'
  ],
  stripe: {
    // Server
-    apiKey: 'sk_test_123', // required
-    apiVersion: '2022-11-15', // optional, default is '2022-11-15'
-    // Client
-    publishableKey: 'pk_test_123', // required
+    server: {
+      key: 'sk_test_123',
+      options: {
+        // your api options override for stripe server side
+        apiVersion: '2022-11-15', // optional, default is '2022-11-15'
+      }
+    // CLIENT
+    },
+    client: {
+      key: 'pk_test_123',
+      // your api options override for stripe client side
+      options: {
+
+      }
+    }
  }
})

But, if accepted, this would need to be reflectedon the env variables, that would break the syntax of the official docs:

-NUXT_PUBLIC_STRIPE_PUBLISHABLE_KEY="pk_live_..."
-NUXT_STRIPE_API_KEY="sk_live_..."
+NUXT_PUBLIC_STRIPE_CLIENT_KEY="pk_live_..."
+NUXT_STRIPE_SERVER_KEY="sk_live_..."

not to mention that client-key could be missleading and associated to something like customer-id or account-key/id.