bloomreach / spa-sdk

Apache License 2.0
15 stars 16 forks source link

relevance-sdk call changes in 21.0.0? #17

Closed HasanKaragulmez closed 11 months ago

HasanKaragulmez commented 1 year ago

Hi, when upgrading to SDK 21.0.0 after 17.1.1 (due to async-issues), notably, https://github.com/bloomreach/spa-sdk/issues/7 - I've now found that the relevance-import example seems broken.

I've got multiple questions regarding this:

1- it seems mandatory that this relevance-sdk call gets executed, due to it setting the _v cookie.

The following error occurs, when trying to import the relevance function with SDK version 21.0.0

info  - Collecting page data .../home/name/projects/website/node_modules/@bloomreach/spa-sdk/lib/express/index.js:1
export { default as relevance } from './relevance.js';
^^^^^^

SyntaxError: Unexpected token 'export'
    at Object.compileFunction (node:vm:360:18)
    at wrapSafe (node:internal/modules/cjs/loader:1088:15)
    at Module._compile (node:internal/modules/cjs/loader:1123:27)
    at Module._extensions..js (node:internal/modules/cjs/loader:1213:10)
    at Module.load (node:internal/modules/cjs/loader:1037:32)
    at Module._load (node:internal/modules/cjs/loader:878:12)
    at Module.require (node:internal/modules/cjs/loader:1061:19)
    at require (node:internal/modules/cjs/helpers:103:18)
    at 8409 (/home/name/projects/website/dist/apps/website/.next/server/pages/_document.js:51:18)
    at __webpack_require__ (/home/name/projects/website/dist/apps/website/.next/server/webpack-runtime.js:25:42)
unhandledRejection Error: Failed to collect page data for /_error
    at /home/name/projects/website/node_modules/next/dist/build/utils.js:1150:15 {
  type: 'Error'
}

The NextJS (which we use) example here also trails behind the current changes, see relevance call here: https://github.com/bloomreach/spa-sdk/blob/e253082582192b5b784be005a1d27e53ef16002e/examples/next/pages/%5B%5B...route%5D%5D.tsx

as it's not updated to use the SDK 21.0.0, which is imperative for us to use do to the async-reverts in 21.0.0.

Is there a new way of calling this, introduced with the cjs changes? What is the correct way?

2- The relevance-function is referenced as only being an Express helper (and we don't use Express, and, neither seems the Bloomreach NextJS example). Is the relevance SDK-call not actually dependent on Express, but always mandatory when supporting Relevance?

joerideg commented 1 year ago

Hi @HasanKaragulmez, We thought we fixed this particular problem, however it seems not, apologies! I already have a fix locally and will release the fix as soon as its been reviewed. I'll keep the issue open until we have verified this solves your problem.

joerideg commented 1 year ago

2- The relevance-function is referenced as only being an Express helper (and we don't use Express, and, neither seems the Bloomreach NextJS example). Is the relevance SDK-call not actually dependent on Express, but always mandatory when supporting Relevance?

No the 'express' module is not actually depending on express. I just provides a function that can be used in middleware on the server. I guess we could rename that path.

joerideg commented 11 months ago

This is fixed in 22.0.x