feat: add support for configuring the base URL #248

fgreinacher · 2022-01-26T11:13:34Z

Description

See the related issue for context.

This PR introduces the following changes:

Sets PUBLIC_URL to a placeholder value during Docker build so create-react-app injects the placeholder value in generated assets (see https://create-react-app.dev/docs/advanced-configuration/ and https://create-react-app.dev/docs/deployment/#building-for-relative-paths)
When the container is started replaces the placeholder value by the configured base URL. If no base URL is configured it will assume being served from root.
Adapt some places where the PUBLIC_URL variable has not been used up to now (see https://create-react-app.dev/docs/using-the-public-folder/)

Related issue(s)

Closes #249

netlify · 2022-01-26T11:13:41Z

✔️ Deploy Preview for modest-rosalind-098b67 ready!

🔨 Explore the source changes: cb46528

🔍 Inspect the deploy log: https://app.netlify.com/sites/modest-rosalind-098b67/deploys/61f28aa9b317d1000834f93a

😎 Browse the preview: https://deploy-preview-248--modest-rosalind-098b67.netlify.app

fgreinacher · 2022-01-26T11:32:39Z

src/index.tsx

+      return './js/monaco/json.worker.bundle.js';
    }
    // for yaml worker
    if (label === 'yaml' || label === 'yml') {
-      return '../../js/monaco/yaml.worker.bundle.js';
+      return './js/monaco/yaml.worker.bundle.js';
    }
    // for core editor worker
-    return '../../js/monaco/editor.worker.bundle.js';
+    return './js/monaco/editor.worker.bundle.js';
  },


I assume the former paths just worked by accident because .. does nothing when already at the root path.

Right, if we load bundled JS file in the index.html then we can use path from root. In the preview I see that it works, but there is a question: why does it also work in current deploy on https://studio.asyncapi.com where we have that old path? 🤔

Also I wonder if we should use it here process.env.PUBLIC_URL to be more safe?

fgreinacher · 2022-01-26T11:34:14Z

@magicmatatjahu We noticed this after deploying the new Docker image from #240 to our Kubernetes cluster. Maybe you can have a look again?

magicmatatjahu · 2022-01-26T12:13:38Z

@fgreinacher

@magicmatatjahu We noticed this after deploying the new Docker image from #240 to our Kubernetes cluster. Maybe you can have a look again?

Hi! No problem, I hope that I will make review today! I already made 4/5 reviews today and nexts wait for me 😅

magicmatatjahu

@fgreinacher Thanks for your contribution! :) We are very thankful but I have a few comments, please refer to them :)

magicmatatjahu · 2022-01-26T13:13:59Z

src/index.tsx

+      return './js/monaco/json.worker.bundle.js';
    }
    // for yaml worker
    if (label === 'yaml' || label === 'yml') {
-      return '../../js/monaco/yaml.worker.bundle.js';
+      return './js/monaco/yaml.worker.bundle.js';
    }
    // for core editor worker
-    return '../../js/monaco/editor.worker.bundle.js';
+    return './js/monaco/editor.worker.bundle.js';
  },


Right, if we load bundled JS file in the index.html then we can use path from root. In the preview I see that it works, but there is a question: why does it also work in current deploy on https://studio.asyncapi.com where we have that old path? 🤔

Dockerfile

magicmatatjahu · 2022-01-26T13:32:26Z

Dockerfile

+COPY package.json package-lock.json ./
+RUN npm install --frozen-lockfile
+
 COPY ./ ./


Why these commands are split? What we have benefit here?

That's a common optimization to speed up Docker builds. It will cache the result of npm install and not execute it unless package.json or package-lock.json have changed.

magicmatatjahu · 2022-01-26T13:34:59Z

Dockerfile

+FROM docker.io/library/nginx:1.21.5-alpine as runtime
+
+ARG BASE_URL_PLACEHOLDER
+ARG ENTRYPOINT_SCRIPT=/docker-entrypoint.d/set-public-url.sh


Could we add info about docker-entrypoint.d? Tbh I had to read about it, because I didn't know about that 😅 You know, for future contributors to help them understand that step.

Yes, sure! I added a comment!

README.md

magicmatatjahu · 2022-01-26T13:40:06Z

src/index.tsx

+      return './js/monaco/json.worker.bundle.js';
    }
    // for yaml worker
    if (label === 'yaml' || label === 'yml') {
-      return '../../js/monaco/yaml.worker.bundle.js';
+      return './js/monaco/yaml.worker.bundle.js';
    }
    // for core editor worker
-    return '../../js/monaco/editor.worker.bundle.js';
+    return './js/monaco/editor.worker.bundle.js';
  },


Also I wonder if we should use it here process.env.PUBLIC_URL to be more safe?

fgreinacher · 2022-01-26T14:17:32Z

Also I wonder if we should use it here process.env.PUBLIC_URL to be more safe?

@magicmatatjahu I cannot comment inline, no idea why... Yes, that makes sense.

I think I addressed everything, much better now!

sonarqubecloud · 2022-01-27T12:06:33Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

magicmatatjahu · 2022-01-27T12:07:07Z

@fgreinacher Ok, I checked and everything works. Thanks very much for contribution! If you will have any problems with new docker image please ping me in this PR :)

magicmatatjahu · 2022-01-27T12:19:44Z

/rtm

asyncapi-bot · 2022-01-27T12:31:14Z

🎉 This PR is included in version 0.7.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

fgreinacher · 2022-01-27T21:11:25Z

Works great now, thanks again for the quick review!

fgreinacher commented Jan 26, 2022

View reviewed changes

fgreinacher marked this pull request as ready for review January 26, 2022 11:35

fgreinacher force-pushed the feat/add-base-url branch from 5d885af to b6bf887 Compare January 26, 2022 11:38

magicmatatjahu requested changes Jan 26, 2022

View reviewed changes

fgreinacher force-pushed the feat/add-base-url branch from b6bf887 to cb0435b Compare January 26, 2022 13:57

feat: add support for configuring the base URL

60d9fa1

fgreinacher force-pushed the feat/add-base-url branch from cb0435b to 60d9fa1 Compare January 26, 2022 14:15

fgreinacher requested a review from magicmatatjahu January 26, 2022 14:17

Merge branch 'master' into feat/add-base-url

cb46528

magicmatatjahu approved these changes Jan 27, 2022

View reviewed changes

asyncapi-bot added ready-to-merge autoapproved labels Jan 27, 2022

github-actions bot approved these changes Jan 27, 2022

View reviewed changes

asyncapi-bot merged commit 53957ef into asyncapi:master Jan 27, 2022

github-actions bot approved these changes Jan 27, 2022

View reviewed changes

asyncapi-bot added the released label Jan 27, 2022

fgreinacher deleted the feat/add-base-url branch January 27, 2022 21:09

KhudaDad414 pushed a commit to KhudaDad414/studio that referenced this pull request Oct 12, 2023

ci: update global workflows (asyncapi#248)

307b132

antoineauger mentioned this pull request Jun 19, 2025

fix: support BASE_URL for studio docker builds #1208

Merged

Uh oh!

feat: add support for configuring the base URL #248

feat: add support for configuring the base URL #248

Uh oh!

Conversation

fgreinacher commented Jan 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify bot commented Jan 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fgreinacher commented Jan 26, 2022

Uh oh!

magicmatatjahu commented Jan 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

magicmatatjahu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fgreinacher commented Jan 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sonarqubecloud bot commented Jan 27, 2022

Uh oh!

magicmatatjahu commented Jan 27, 2022

Uh oh!

magicmatatjahu commented Jan 27, 2022

Uh oh!

asyncapi-bot commented Jan 27, 2022

Uh oh!

fgreinacher commented Jan 27, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fgreinacher commented Jan 26, 2022 •

edited

Loading

netlify bot commented Jan 26, 2022 •

edited

Loading

magicmatatjahu commented Jan 26, 2022 •

edited

Loading

fgreinacher commented Jan 26, 2022 •

edited

Loading