feat: add-api-proxy: Add Nitro server API routes #179

vloss3 · 2023-10-19T11:22:02Z

Solves #165

src/module.ts

src/runtime/composables/useDrupalCe.ts

src/module.ts

TurtlBbx · 2023-10-19T14:03:31Z

src/runtime/composables/useDrupalCe.ts

@@ -12,7 +12,11 @@ export const useDrupalCe = () => {
   * @returns UseFetchOptions<any>
   */
  const processFetchOptions = (fetchOptions:UseFetchOptions<any> = {}) => {
-    fetchOptions.baseURL = fetchOptions.baseURL ?? config.baseURL
+    if (config.exposeAPIRouteRules) {
+      fetchOptions.baseURL = useRequestURL().origin + '/api/drupal-ce'


Also, if this function is called from fetchMenu, then instead of '/api/drupal-ce', baseURL should be '/api/menu'. That way, we make use of the menu proxy defined in routeRules.
I'm fine with using just one simple flag, e.g. isFetchMenu, to know when to apply '/api/menu'.

a path prefix and a baes url are two different things. maybe the code should be re-factored into a helper. different topic.

TurtlBbx · 2023-10-19T14:08:06Z

@vloss3 Once the improvements are done, could you add a test case for when the exposeAPIRouteRules option is set to false. It can be a copy of https://github.com/drunomics/nuxtjs-drupal-ce/blob/2.x/test/basic.test.ts with the exposeAPIRouteRules set to false.

TurtlBbx · 2023-10-20T06:14:37Z

@vloss3 README needs to be updated as well for this feature.

src/runtime/composables/useDrupalCe.ts

README.md

src/module.ts

README.md

vloss3 · 2023-11-21T08:17:22Z

The following was discussed to move forward with this feature:

New options:

drupalBaseUrl: origin URL, for iframes and other cases, changeable via env variable
serverDrupalBaseUrl: http://node (optional: use this during server render if set))
ceApiEndpoint: /ce-api (default)
menuEndpoint: unchanged
menuBaseUrl: http://domain.com/ce-api (default: drupalUrl + ceApiEndpoint), changeable via env variable

Note: Those options will have defaults and will not require configuration changes

Route rules changes:

remove /api/drupal
convert route rules to nitro server routes

Use cases to consider:

proxy files through frontend
add an iframe of drupal in the frontend project code

Tests and customizing cache rules(by project) will be carried in follow-up issues.

fago · 2023-12-05T14:25:27Z

@vloss3 I updated README slightly. I was not sure about DRUPAL_BASE_URL - do we still support that variable actually? Since there is built-in support in nuxt to override config via NUXT_PUBLIC_... I could see us dropping that + documenting that instead.

src/runtime/composables/useDrupalCe.ts

…ides

README.md

src/runtime/server/api/menu.ts

README.md

fago · 2024-01-01T22:06:29Z

README.md

+
+Runtime config values can be overridden with environment variables via `NUXT_PUBLIC_` prefix. Supported runtime overrides:
+
+- `baseUrl` -> `NUXT_PUBLIC_DRUPAL_CE_BASE_URL`


this documents an override for the deprecated optiln. it should document it for drupalbaseUrl

@fago @vloss3 In code, we don't support ENV override of drupalBaseUrl. It's not taken into account, as seen here.

We either:
a) Change back in README to state that we support baseUrl (and not drupalBaseUrl) override (it's used in the server proxy handler).
b) Change the code to support drupalbaseUrl override and leave the README as-is.

We need to have that kind of override working, this is critical functionaliaty, we cannot merge without it.

The module had code merge in runtimeconfig and make it override regular config before. maybe this is still there and working?

fixed drupalBaseUrl to work with env overrides

fago · 2024-01-01T22:08:36Z

src/runtime/server/api/menu.ts

+
+export default defineEventHandler(async (event) => {
+  const drupalCe = useRuntimeConfig().public.drupalCe
+  const menuBaseUrl = drupalCe.serverDrupalBaseUrl || drupalCe.menuBaseUrl


it should primarily use the configured menuBaseUrl and fallback to serverDrupalBaseURL.

src/module.ts

fago · 2024-01-10T10:16:33Z

src/runtime/server/api/drupalCe.ts

+export default defineEventHandler(async (event) => {
+  const path = `/${event.context.params._.replace('api/drupal-ce', '')}`
+  const drupalCe = useRuntimeConfig().public.drupalCe
+  const drupalBaseUrl = drupalCe.serverDrupalBaseUrl || drupalCe.baseURL


this is a correct fallback which is missing at the menu route

fago · 2024-01-10T10:16:51Z

src/runtime/server/api/menu.ts

+
+export default defineEventHandler(async (event) => {
+  const drupalCe = useRuntimeConfig().public.drupalCe
+  const menuBaseUrl = drupalCe.menuBaseUrl || drupalCe.serverDrupalBaseUrl


serverDrupalBaseUrl is not required, so not enough as fallback

TurtlBbx · 2024-01-18T10:17:06Z

src/runtime/server/api/menu.ts

+
+export default defineEventHandler(async (event) => {
+  const drupalCe = useRuntimeConfig().public.drupalCe
+  const menuBaseUrl = drupalCe.serverDrupalBaseUrl || drupalCe.menuBaseUrl


@vloss3 This is wrong I think:
const menuBaseUrl = drupalCe.serverDrupalBaseUrl || drupalCe.menuBaseUrl
It should be:
const menuBaseUrl = drupalCe.menuBaseUrl || (drupalCe.serverDrupalBaseUrl || drupalCe.drupalBaseUrl) + drupalCe.ceApiEndpoint
Because serverDrupalBaseUrl and drupalBaseUrl contains no ceApiEndpoint postfix.

README.md

fago · 2024-01-10T10:21:01Z

src/module.ts

-import { defineNuxtModule, addPlugin, createResolver, addImportsDir } from '@nuxt/kit'
-import { UseFetchOptions } from 'nuxt/dist/app/composables'
+import { defineNuxtModule, addPlugin, createResolver, addImportsDir, addServerHandler } from '@nuxt/kit'
+import type { UseFetchOptions } from 'nuxt/dist/app/composables'
 import { defu } from 'defu'

 export interface ModuleOptions {
  baseURL: string,


this is optional actually, since deprecated

fago · 2024-01-10T10:21:19Z

src/module.ts

 import { defu } from 'defu'

 export interface ModuleOptions {
  baseURL: string,
+  drupalBaseUrl?: string,


this is required now. can we mark it so or will that be a BC-issue?

fago · 2024-01-18T22:07:12Z

src/runtime/server/api/drupalCe.ts

+import { useRuntimeConfig } from '#imports'
+
+export default defineEventHandler(async (event) => {
+  const path = `/${event.context.params._.replace('api/drupal-ce', '')}`


hard to read. better use '/' + ...
also, it's not safe. it should remove the prefix characters only, e.g. a fixed not number of chars - not remove any occurrences of this string.

fago · 2024-01-18T22:08:07Z

src/runtime/server/api/menu.ts

+export default defineEventHandler(async (event) => {
+  const { serverDrupalBaseUrl, drupalBaseUrl, menuBaseUrl, ceApiEndpoint } = useRuntimeConfig().public.drupalCe
+  const menuUrl = menuBaseUrl || (serverDrupalBaseUrl || drupalBaseUrl) + ceApiEndpoint
+  const menu = event.context.params._


is that normal? _ parameter is very very strange and not readable. does it have no proper API to get a parameter in readable away?

there is getRouterParams(event) which also returns _, will use this method

fago · 2024-01-18T22:09:26Z

src/runtime/server/api/menu.ts

+
+export default defineEventHandler(async (event) => {
+  const { serverDrupalBaseUrl, drupalBaseUrl, menuBaseUrl, ceApiEndpoint } = useRuntimeConfig().public.drupalCe
+  const menuUrl = menuBaseUrl || (serverDrupalBaseUrl || drupalBaseUrl) + ceApiEndpoint


minor: menuBaseUrl.
the menu url would already incldue the menu name

fago · 2024-01-18T22:10:34Z

src/runtime/composables/useDrupalCe.ts

@@ -101,6 +109,10 @@ export const useDrupalCe = () => {
    const baseMenuPath = config.menuEndpoint.replace('$$$NAME$$$', name)
    const menuPath = ref(baseMenuPath)

+    if (config.exposeAPIRouteRules) {
+      useFetchOptions.baseURL = useRequestURL().origin + '/api/menu'


hm, I do still think this is problematic. if run on a CDN, the original url will be the wrong one.

I don't think it has to be the original url as the server route request gets forwarded as well:
cdn-domain.com/api/drupal-ce -> localhost:3000/api/drupal-ce

so it seems to work? I've tested this with a nginx reverse proxy behind Cloudflare

fago · 2024-01-18T22:13:15Z

src/runtime/composables/useDrupalCe.ts

@@ -12,7 +12,11 @@ export const useDrupalCe = () => {
   * @returns UseFetchOptions<any>
   */
  const processFetchOptions = (fetchOptions:UseFetchOptions<any> = {}) => {
-    fetchOptions.baseURL = fetchOptions.baseURL ?? config.baseURL
+    if (config.exposeAPIRouteRules) {
+      fetchOptions.baseURL = useRequestURL().origin + '/api/drupal-ce'


a path prefix and a baes url are two different things. maybe the code should be re-factored into a helper. different topic.

src/runtime/composables/useDrupalCe.ts

vloss3 added 3 commits October 19, 2023 14:20

feat: add-api-proxy: Add Nitro server API routes

95e6897

Remove debug code

0486f4a

Fix tests

dc67d86

TurtlBbx reviewed Oct 19, 2023

View reviewed changes

src/module.ts Outdated Show resolved Hide resolved

TurtlBbx reviewed Oct 19, 2023

View reviewed changes

src/runtime/composables/useDrupalCe.ts Outdated Show resolved Hide resolved

TurtlBbx reviewed Oct 19, 2023

View reviewed changes

src/module.ts Outdated Show resolved Hide resolved

TurtlBbx reviewed Oct 19, 2023

View reviewed changes

vloss3 added 2 commits October 26, 2023 15:04

Fix menu route rules and add docs

0817411

Add test for disabled exposeAPIRouteRules

1117aad

TurtlBbx reviewed Oct 26, 2023

View reviewed changes

src/runtime/composables/useDrupalCe.ts Outdated Show resolved Hide resolved

vloss3 added 3 commits October 26, 2023 18:47

Improve docs

d3927a8

Improve docs

1d6c9a6

Merge remote-tracking branch 'origin/2.x' into feature/165-add-api-proxy

cca49d1

TurtlBbx approved these changes Oct 27, 2023

View reviewed changes

TurtlBbx added 2 commits October 27, 2023 12:02

Fix module build error.

72220f4

Fix menu proxy not working.

8409bce

fago requested changes Oct 31, 2023

View reviewed changes

README.md Outdated Show resolved Hide resolved

src/module.ts Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

add-api-proxy: Add README and add BC baseUrl

003b311

add-api-proxy: Replace routeRules with server handlers

bee448c

vloss3 force-pushed the feature/165-add-api-proxy branch from 3f77d1f to bee448c Compare December 6, 2023 19:10

vloss3 added 3 commits December 6, 2023 20:31

add-api-proxy: Remove console log

1bf7fc8

add-api-proxy: Handle cases where baseURL is not full URL

da133ce

add-api-proxy: Add back README commit

f454685

fago requested changes Dec 12, 2023

View reviewed changes

src/runtime/composables/useDrupalCe.ts Outdated Show resolved Hide resolved

add-api-proxy: Support i18n in menus and add README for runtime overr…

82c5d53

…ides

TurtlBbx reviewed Dec 12, 2023

View reviewed changes

README.md Outdated Show resolved Hide resolved

TurtlBbx reviewed Dec 12, 2023

View reviewed changes

src/runtime/server/api/menu.ts Outdated Show resolved Hide resolved

add-api-proxy: Add override examples and use serverDrupalBaseUrl

c9feacf

TurtlBbx reviewed Dec 13, 2023

View reviewed changes

README.md Outdated Show resolved Hide resolved

vloss3 and others added 6 commits December 13, 2023 18:03

add-api-proxy: Improve README

4384241

Improve README

ad8d084

Fix a build error

24b5ec9

Fix build errors

03ea4cd

Fix menu endpoint proxy

f0352c4

Improve README

51a8ac6

TurtlBbx approved these changes Dec 14, 2023

View reviewed changes

Fix default options

401a753

TurtlBbx approved these changes Dec 14, 2023

View reviewed changes

fago added 2 commits December 15, 2023 12:19

Update README.md

2ea6a68

correctly test BC

8c098c4

fago requested changes Jan 1, 2024

View reviewed changes

add-api-proxy: Update README and menu fallback

80908d2

fago requested changes Jan 10, 2024

View reviewed changes

vloss3 added 3 commits January 17, 2024 13:47

Merge remote-tracking branch 'origin/2.x' into feature/165-add-api-proxy

a3940c4

add-api-proxy: Allow env overrides for drupalBaseUrl

f5a39ae

add-api-proxy: Update vitest

2ac578f

TurtlBbx reviewed Jan 18, 2024

View reviewed changes

README.md Show resolved Hide resolved

add-api-proxy: Improve menu route and README

476d1cd

TurtlBbx approved these changes Jan 18, 2024

View reviewed changes

fago requested changes Jan 18, 2024

View reviewed changes

vloss3 added 3 commits January 22, 2024 16:53

add-api-proxy: Small refactor

07f0064

add-api-proxy: Refactor menu api code

d1ab251

add-api-proxy: Add handler for base path

cf1dc10

vloss3 merged commit af80b10 into 2.x Jan 31, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add-api-proxy: Add Nitro server API routes #179

feat: add-api-proxy: Add Nitro server API routes #179

vloss3 commented Oct 19, 2023 •

edited

Loading

TurtlBbx Oct 19, 2023

fago Jan 18, 2024

TurtlBbx commented Oct 19, 2023

TurtlBbx commented Oct 20, 2023

vloss3 commented Nov 21, 2023 •

edited

Loading

fago commented Dec 5, 2023

fago Jan 1, 2024

TurtlBbx Jan 9, 2024 •

edited

Loading

fago Jan 10, 2024

vloss3 Jan 17, 2024

fago Jan 1, 2024

fago Jan 10, 2024

fago Jan 10, 2024

TurtlBbx Jan 18, 2024

fago Jan 10, 2024

fago Jan 10, 2024

fago Jan 18, 2024

fago Jan 18, 2024

vloss3 Jan 19, 2024

fago Jan 18, 2024

fago Jan 18, 2024

vloss3 Jan 22, 2024

fago Jan 18, 2024


		Runtime config values can be overridden with environment variables via `NUXT_PUBLIC_` prefix. Supported runtime overrides:

		- `baseUrl` -> `NUXT_PUBLIC_DRUPAL_CE_BASE_URL`

feat: add-api-proxy: Add Nitro server API routes #179

feat: add-api-proxy: Add Nitro server API routes #179

Conversation

vloss3 commented Oct 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TurtlBbx commented Oct 19, 2023

TurtlBbx commented Oct 20, 2023

vloss3 commented Nov 21, 2023 • edited Loading

fago commented Dec 5, 2023

Choose a reason for hiding this comment

TurtlBbx Jan 9, 2024 • edited Loading

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vloss3 commented Oct 19, 2023 •

edited

Loading

vloss3 commented Nov 21, 2023 •

edited

Loading

TurtlBbx Jan 9, 2024 •

edited

Loading