feat: add pass headers guide #6960

rohrig · 2023-09-19T15:46:48Z

🔗 Linked issue

❓ Type of change

[x ] 📖 Documentation (updates to the documentation, readme or JSDoc annotations)

📚 Description

A guide to show integrators how to pass headers from the frontend through to the external services

📝 Checklist

I have linked an issue or discussion.
I have updated the documentation accordingly.

mattmaribojoc

Looks good, very minor consistency suggestions

docs/content/5.integrations/2.guide/2.adding-headers-to-request.md

filrak

Maybe I got something wrong but since we're using Nuxt boilerplate shouldn't the guide focus more on how to modify the existing code for this particular use case rather than explaining how to set up the SDK itself?

docs/content/5.integrations/2.guide/2.adding-headers-to-request.md

rohrig · 2023-10-02T09:28:42Z

Maybe I got something wrong but since we're using Nuxt boilerplate shouldn't the guide focus more on how to modify the existing code for this particular use case rather than explaining how to set up the SDK itself?

The guide does not use the nuxt 3 boilerplate. It uses the integrations boilerplate with the nuxt option.

…t.md Co-authored-by: Matt Maribojoc <mmaribojoc@vuestorefront.io>

sonarcloud · 2023-10-26T15:11:03Z

[vuestorefront_vue-storefront_sdk] Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

sonarcloud · 2023-10-26T15:11:16Z

[vuestorefront_vue-storefront_middleware] Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

sonarcloud · 2023-10-26T15:11:17Z

[vuestorefront_vue-storefront_cli] Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

Wojtek asked me to review the docs and correct it. This is not the final version yet.

lsliwaradioluz · 2023-12-20T10:29:34Z

@rohrig, here are my suggestions: https://github.com/vuestorefront/vue-storefront/pull/6996/files

LMK if you need any help with understanding those!

…uide-lsliwa feat: add pass headers guide (suggestions)

michaelKurowski · 2023-12-20T11:47:52Z

docs/content/3.cookbook/2.pass-custom-headers-and-cookies-to-requests.md

+```javascript
+import { initSDK, buildModule } from '@vue-storefront/sdk';
+import { client, boilerplateModule, BoilerplateModuleType } from '../../packages/sdk/src';
+import useHeaders from 'path-to-useHeaders-file'; // Replace with the correct path to the `useHeaders` composable.


I don't understand, who does provide the useHeaders, it's our product we should know the path to the composable if we provide it 🤔

It also feels like it's strictly vue specific since it mentions composables, is this what we want? I don't see any references to vue in the path of this file so it seems like you provide the general advice

I see that the customer is responsible for creation of the composable, but my question regarding the vue-centric docs remain.

I agree, if we can create a solution that framework agnostic that would be ideal. But I think given the differences in how Next and Nuxt handle ssr/state, I'm not sure that's possible. I'm not familiar enough with Next to know.

For context, the guide was made in response to an integrator's query that was using Nuxt. This started as general enablement and turned into a guide when several more folks asked the same question and all were using Nuxt.

If it's not possible to provide a framework agnostic solution, we can at least add examples in both support frameworks.

I'm ok for both frameworks! :) Just including nuxt only solution felt a bit off

Fifciu

How is this whole guide related to risks of shared client mentioned by @bartoszherba recently? Namely.

Fifciu · 2023-12-20T11:13:21Z

docs/content/3.cookbook/2.pass-custom-headers-and-cookies-to-requests.md

+
+```javascript
+import { initSDK, buildModule } from '@vue-storefront/sdk';
+import { client, boilerplateModule, BoilerplateModuleType } from '../../packages/sdk/src';


Is it proper path?

Fifciu · 2023-12-20T11:16:30Z

docs/content/3.cookbook/2.pass-custom-headers-and-cookies-to-requests.md

+```javascript
+import { initSDK, buildModule } from '@vue-storefront/sdk';
+import { client, boilerplateModule, BoilerplateModuleType } from '../../packages/sdk/src';
+import useHeaders from 'path-to-useHeaders-file'; // Replace with the correct path to the `useHeaders` composable.


Do we provide it for our storefronts?

Ah it's below

Probably we should make it more clear

Fifciu · 2023-12-20T11:37:28Z

docs/content/3.cookbook/2.pass-custom-headers-and-cookies-to-requests.md

+  if (interceptorId !== null) {
+    client.interceptors.request.eject(interceptorId);
+  }
+
+  interceptorId = client.interceptors.request.use(


I guess it will become flaky and unpredictable when there are many parallel connections to applications and shared client

Could you elaborate? Not sure about the dangers if somebody uses the SDK server-side (e.g. in an Express application) but when it comes to the usage inside a storefront, how can SDK client be shared across multiple users running the storefront in their own browser? 🤔

I confirm that's the case.

Fifciu · 2023-12-20T11:48:22Z

docs/content/3.cookbook/2.pass-custom-headers-and-cookies-to-requests.md

+  name: 'extension-dev-headers',
+  hooks: (req, res) => {
+    return {
+      afterCreate: ({ configuration }) => {


Why afterCreate instead of beforeCall?

I believe it could be both

michaelKurowski · 2023-12-20T12:04:51Z

docs/content/3.cookbook/2.pass-custom-headers-and-cookies-to-requests.md

+```javascript
+export default function () {
+  const serverHeaders = useRequestHeaders(['cookie']);
+  const devModeCookie = useCookie('dev-mode')?.value || '';
+
+  const state = useState<{ headerData: null | { isDevMode: string }, loading: boolean }>('headerState', () => ({
+    headerData: null,
+    loading: false,
+  }));
+
+  function addHeadersToState() {
+    getDevModeHeader(devModeCookie, serverHeaders.cookie || '');
+  }
+
+  function getHeaderObj(headers: string): { [key: string]: string } {
+    if (!headers) return {};
+
+    return headers.split(';').reduce<{ [key: string]: string }>((acc, item) => {
+      const parts = item.trim().split('=');
+      if (parts.length === 2) {
+        const [key, value] = parts;
+        acc[key] = value;
+      }
+      return acc;
+    }, {});
+  }
+
+  function getDevModeHeader(cookieHeader: string, serverHeader: string) {
+    if (process.server) {
+      const headerObj = getHeaderObj(serverHeader);
+      state.value.headerData = { isDevMode: headerObj['dev-mode'] || '' };
+      return
+    }
+
+    state.value.headerData = { isDevMode: cookieHeader };
+  }
+
+  return {
+    addHeadersToState,
+    ...toRefs(state.value),
+  }
+}


Should we provide it ootb, instead of asking user to write it themselves? We even have the code already, you just wrote it.

I think that would be great. But I created the example quickly and I'm not certain there aren't edge cases that need to be addressed. This PR is a bit old, so Unified wasn't on my radar when I wrote the guide and we didn't have a storefront in which I could add it. But now we do, so I think it would indeed be great to have this ootb in Unified.

@jagoral wdyt

michaelKurowski · 2023-12-20T12:08:27Z

docs/content/3.cookbook/2.pass-custom-headers-and-cookies-to-requests.md

+
+```javascript
+import { initSDK, buildModule } from '@vue-storefront/sdk';
+import { client, boilerplateModule, BoilerplateModuleType } from '../../packages/sdk/src';


I don't understand, where is the client coming from, is it exposed by every SDK module? Is it coordinated with the unified team, do they expose it as well the same way?

This import path should be less ambigious. But long story short: the client is coming from an SDK module (e.g. sapcc-sdk), every module exports the client. Not sure this is going to be the case in a few weeks/months (considering the ongoing discussion initiated by @bartoszherba) but, for now, that's the state of things.

I really think this doc should be agreed upon with unified. Consider how confused a customer on unified storefront would be after seeing these docs if unified module doesn't expose this (and honestly they probably couldn't know before that they should).

This guide sat on the back burner for a while. Given the change in landscape since it's inception I agree there should be a consensus and standard way of doing this. RE: comments about ootb, I think it would be sensible for for the Unified team to implement the boilerplate to allow passing custom headers and allow the end dev to use a one-liner, if possible, to add headers to requests. The guide for an ootb solution would be a separate guide though.

we still have two scenarios:

Projects using Unified: for those we can have an ootb solution and a separate guide

Non-Unified: this sort of guide will still be necessary.

WojtekTheWebDev

exporting client as our singleton instance became problematic recently and I think we should discuss it during the refi before releasing this guide

rohrig requested review from mattmaribojoc and skirianov September 19, 2023 15:47

mattmaribojoc approved these changes Sep 19, 2023

View reviewed changes

docs/content/5.integrations/2.guide/2.adding-headers-to-request.md Outdated Show resolved Hide resolved

docs/content/5.integrations/2.guide/2.adding-headers-to-request.md Outdated Show resolved Hide resolved

skirianov approved these changes Sep 20, 2023

View reviewed changes

filrak requested changes Sep 20, 2023

View reviewed changes

This comment was marked as outdated.

Sign in to view

rohrig requested a review from filrak October 2, 2023 12:21

rohrig and others added 7 commits October 26, 2023 17:04

feat: add pass headers guide

fea83b2

Update docs/content/5.integrations/2.guide/2.adding-headers-to-reques…

8df509c

…t.md Co-authored-by: Matt Maribojoc <mmaribojoc@vuestorefront.io>

add cookbook and move header guide to cookbook

af7c294

typo fix

3c4508b

feat: remove warning

fd2959e

fix: typo

882068f

chore: remove unneed prerequisites

b5e7541

rohrig force-pushed the feat/pass-custom-header-guide branch from f8756fd to b5e7541 Compare October 26, 2023 15:05

refactor: intermediate changes

8c61597

Wojtek asked me to review the docs and correct it. This is not the final version yet.

rohrig and others added 4 commits December 20, 2023 11:37

Merge pull request #6996 from vuestorefront/feat/pass-custom-header-g…

83ecaa7

…uide-lsliwa feat: add pass headers guide (suggestions)

Merge branch 'main' into feat/pass-custom-header-guide

2730ea6

fix: steps not rendering - use plain text

cf6a36b

fix: format warning for better legibility

93d6ae1

michaelKurowski reviewed Dec 20, 2023

View reviewed changes

Fifciu requested changes Dec 20, 2023

View reviewed changes

michaelKurowski reviewed Dec 20, 2023

View reviewed changes

WojtekTheWebDev requested changes Dec 20, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add pass headers guide #6960

feat: add pass headers guide #6960

rohrig commented Sep 19, 2023

mattmaribojoc left a comment

filrak left a comment

This comment was marked as outdated.

rohrig commented Oct 2, 2023

sonarcloud bot commented Oct 26, 2023

sonarcloud bot commented Oct 26, 2023

sonarcloud bot commented Oct 26, 2023

lsliwaradioluz commented Dec 20, 2023

michaelKurowski Dec 20, 2023 •

edited

michaelKurowski Dec 20, 2023 •

edited

michaelKurowski Dec 20, 2023

rohrig Dec 20, 2023

michaelKurowski Dec 21, 2023

Fifciu left a comment

Fifciu Dec 20, 2023

Fifciu Dec 20, 2023

Fifciu Dec 20, 2023

Fifciu Dec 20, 2023

Fifciu Dec 20, 2023

lsliwaradioluz Dec 20, 2023

lsliwaradioluz Dec 20, 2023 •

edited

Fifciu Dec 20, 2023

lsliwaradioluz Dec 20, 2023

michaelKurowski Dec 20, 2023 •

edited

rohrig Dec 20, 2023

michaelKurowski Dec 21, 2023

michaelKurowski Dec 20, 2023

lsliwaradioluz Dec 20, 2023

michaelKurowski Dec 20, 2023 •

edited

rohrig Dec 20, 2023

WojtekTheWebDev left a comment

feat: add pass headers guide #6960

Are you sure you want to change the base?

feat: add pass headers guide #6960

Conversation

rohrig commented Sep 19, 2023

🔗 Linked issue

❓ Type of change

📚 Description

📝 Checklist

mattmaribojoc left a comment

Choose a reason for hiding this comment

filrak left a comment

Choose a reason for hiding this comment

This comment was marked as outdated.

rohrig commented Oct 2, 2023

sonarcloud bot commented Oct 26, 2023

sonarcloud bot commented Oct 26, 2023

sonarcloud bot commented Oct 26, 2023

lsliwaradioluz commented Dec 20, 2023

michaelKurowski Dec 20, 2023 • edited

Choose a reason for hiding this comment

michaelKurowski Dec 20, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Fifciu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lsliwaradioluz Dec 20, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelKurowski Dec 20, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelKurowski Dec 20, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WojtekTheWebDev left a comment

Choose a reason for hiding this comment

michaelKurowski Dec 20, 2023 •

edited

michaelKurowski Dec 20, 2023 •

edited

lsliwaradioluz Dec 20, 2023 •

edited

michaelKurowski Dec 20, 2023 •

edited

michaelKurowski Dec 20, 2023 •

edited