Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: adapt base url to netlify #784

Merged
merged 4 commits into from
Nov 28, 2023
Merged

fix: adapt base url to netlify #784

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8d5bf23
fix: adapt base url to netlify
milljoniaer Nov 28, 2023
c189f4b
fix: tests
milljoniaer Nov 28, 2023
fe5d924
fix: netlify constants
milljoniaer Nov 28, 2023
714b21a
chore: remove comment
milljoniaer Nov 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions gatsby/config-options/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { buildGatsbyCloudPreviewUrl } from '../util/build-gatsby-cloud-preview-url';
import { buildNetlifyPreviewUrl } from '../util/build-netlify-preview-url';
export const RSS_FEED_URL = '/blog/rss.xml';
export const DEFAULT_META_IMAGE_URL_PATH = '/sy-share-image.jpg';
export const GATSBY_SITE_PREFIX = process.env.GATSBY_SITE_PREFIX || '';
export const BRANCH_PREVIEW_URL = buildGatsbyCloudPreviewUrl({
prefix: GATSBY_SITE_PREFIX,
export const NETLIFY_DOMAIN_NAME = process.env.GATSBY_NETLIFY_DOMAIN_NAME || '';
milljoniaer marked this conversation as resolved.
Show resolved Hide resolved
export const BRANCH_PREVIEW_URL = buildNetlifyPreviewUrl({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the preview being null is mostly the case for non-preview environments. It will then use GATBSY_BASE_URL (following line). Is this something we create or given by netlify? Asking to make sure this will work when we merge it, because right now I can only verify the preview part of this PR and I fear it's not working later on production. Do you have confidence that this will work then? Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The GATSBY_BASE_URL (and also the GATSBY_PREFIX which I changed to GATSBY_NETLIFY_DOMAIN_NAME) was part of our environment configuration on gatsby cloud and got lost on the migration to netlify. I readded it in the netlify settings. Hence, it currently works again on production. This PR only fixes the feature branch BASE_URL. for the main branch the BRANCH_PREVIEW_URL will still be "null" and the GATSBY_BASE_URL will be used. And since that works currently on production, I am confident, that it will still work once this PR is merged. But I will monitor it.

domainName: NETLIFY_DOMAIN_NAME,
branch: process.env.BRANCH,
reviewId: process.env.REVIEW_ID,
});

// either use a branch preview url if any
Expand Down
35 changes: 0 additions & 35 deletions gatsby/util/build-gatsby-cloud-preview-url.test.ts
View file
Open in desktop

This file was deleted.

23 changes: 0 additions & 23 deletions gatsby/util/build-gatsby-cloud-preview-url.ts
View file
Open in desktop

This file was deleted.

30 changes: 30 additions & 0 deletions gatsby/util/build-netlify-preview-url.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { buildNetlifyPreviewUrl } from './build-netlify-preview-url';

describe('build-netlify-preview-url', () => {
test('should return a valid url', () => {
const url = buildNetlifyPreviewUrl({
domainName: 'satellytes',
branch: 'feature-branch-1',
reviewId: '123',
});
expect(url).toBe('https://deploy-preview-123--satellytes.netlify.app/');
});

test('should return null for the production branch', () => {
const url = buildNetlifyPreviewUrl({
domainName: 'satellytescommain',
branch: 'main',
reviewId: '123',
});
expect(url).toBe(null);
});

test('should return null if no domainName is given', () => {
const url = buildNetlifyPreviewUrl({
domainName: null,
branch: 'something',
reviewId: '123',
});
expect(url).toBe(null);
});
});
8 changes: 8 additions & 0 deletions gatsby/util/build-netlify-preview-url.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
const PRODUCTION_BRANCH = 'main';
export const buildNetlifyPreviewUrl = ({ domainName, reviewId, branch }) => {
if (!domainName || !reviewId || branch === PRODUCTION_BRANCH) {
return null;
}

return `https://deploy-preview-${reviewId}--${domainName}.netlify.app/`;
};