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.

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(build-info): fix detection of Remix and Hydrogen sites #5820

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 14 additions & 5 deletions packages/build-info/src/frameworks/framework.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,17 @@ export interface Framework {
id: string
name: string
category: Category
/**
* If this is set, at least ONE of these must exist, anywhere in the project
*/
configFiles: string[]
/**
* If this is set, at least ONE of these must be present in the `package.json` `dependencies|devDependencies`
*/
npmDependencies: string[]
/**
* if this is set, NONE of these must be present in the `package.json` `dependencies|devDependencies`
*/
excludedNpmDependencies?: string[]
version?: SemVer
/** Information on how it was detected and how accurate the detection is */
Expand Down Expand Up @@ -255,9 +264,9 @@ export abstract class BaseFramework implements Framework {
}

/** detect if the framework config file is located somewhere up the tree */
private async detectConfigFile(): Promise<Detection | undefined> {
if (this.configFiles?.length) {
const config = await this.project.fs.findUp(this.configFiles, {
protected async detectConfigFile(configFiles: string[]): Promise<Detection | undefined> {
if (configFiles.length) {
const config = await this.project.fs.findUp(configFiles, {
cwd: this.path || this.project.baseDirectory,
stopAt: this.project.root,
})
Expand All @@ -275,7 +284,7 @@ export abstract class BaseFramework implements Framework {

/**
* Checks if the project is using a specific framework:
* - if `npmDependencies` is set, one of them must be present in then `package.json` `dependencies|devDependencies`
* - if `npmDependencies` is set, one of them must be present in the `package.json` `dependencies|devDependencies`
* - if `excludedNpmDependencies` is set, none of them must be present in the `package.json` `dependencies|devDependencies`
* - if `configFiles` is set, one of the files must exist
*/
Expand All @@ -286,7 +295,7 @@ export abstract class BaseFramework implements Framework {
}

const npm = await this.detectNpmDependency()
const config = await this.detectConfigFile()
const config = await this.detectConfigFile(this.configFiles ?? [])
this.detected = mergeDetections([npm, config])

if (this.detected) {
Expand Down
86 changes: 86 additions & 0 deletions packages/build-info/src/frameworks/hydrogen.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import { beforeEach, expect, test } from 'vitest'

import { mockFileSystem } from '../../tests/mock-file-system.js'
import { NodeFS } from '../node/file-system.js'
import { Project } from '../project.js'

beforeEach((ctx) => {
ctx.fs = new NodeFS()
})

test('detects a Hydrogen v2 site using the Remix Classic Compiler', async ({ fs }) => {
const cwd = mockFileSystem({
'remix.config.js': '',
'package.json': JSON.stringify({
scripts: {
build: 'remix build',
dev: 'remix dev --manual -c "netlify dev"',
preview: 'netlify serve',
},
dependencies: {
'@netlify/edge-functions': '^2.2.0',
'@netlify/remix-edge-adapter': '^3.1.0',
'@netlify/remix-runtime': '^2.1.0',
'@remix-run/react': '^2.2.0',
'@shopify/cli': '3.50.0',
'@shopify/cli-hydrogen': '^6.0.0',
'@shopify/hydrogen': '^2023.10.0',
react: '^18.2.0',
'react-dom': '^18.2.0',
},
devDependencies: {
'@remix-run/dev': '^2.2.0',
},
}),
})
const detected = await new Project(fs, cwd).detectFrameworks()

const detectedFrameworks = (detected ?? []).map((framework) => framework.id)
expect(detectedFrameworks).not.toContain('remix')
expect(detectedFrameworks).not.toContain('vite')

expect(detected?.[0]?.id).toBe('hydrogen')
expect(detected?.[0]?.build?.command).toBe('remix build')
expect(detected?.[0]?.build?.directory).toBe('public')
expect(detected?.[0]?.dev?.command).toBe('remix dev --manual -c "netlify dev"')
expect(detected?.[0]?.dev?.port).toBeUndefined()
})

test('detects a Hydrogen v2 site using Remix Vite', async ({ fs }) => {
const cwd = mockFileSystem({
'vite.config.ts': '',
'package.json': JSON.stringify({
scripts: {
build: 'remix vite:build',
dev: 'shopify hydrogen dev --codegen',
preview: 'netlify serve',
},
dependencies: {
'@netlify/edge-functions': '^2.10.0',
'@netlify/remix-edge-adapter': '^3.4.0',
'@netlify/remix-runtime': '^2.3.0',
'@remix-run/react': '^2.11.2',
'@shopify/hydrogen': '^2024.7.4',
react: '^18.2.0',
'react-dom': '^18.2.0',
},
devDependencies: {
'@remix-run/dev': '^2.11.2',
'@shopify/cli': '^3.66.1',
'@shopify/hydrogen-codegen': '^0.3.1',
vite: '^5.4.3',
},
}),
})
const detected = await new Project(fs, cwd).detectFrameworks()

const detectedFrameworks = (detected ?? []).map((framework) => framework.id)
expect(detectedFrameworks).not.toContain('remix')
expect(detectedFrameworks).not.toContain('vite')

expect(detected?.[0]?.id).toBe('hydrogen')
expect(detected?.[0]?.build?.command).toBe('remix vite:build')
expect(detected?.[0]?.build?.directory).toBe('dist/client')
expect(detected?.[0]?.dev?.command).toBe('shopify hydrogen dev')
expect(detected?.[0]?.dev?.port).toBe(5173)
})
64 changes: 52 additions & 12 deletions packages/build-info/src/frameworks/hydrogen.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,65 @@
import { BaseFramework, Category, Framework } from './framework.js'
import { BaseFramework, Category, DetectedFramework, Framework } from './framework.js'

const CLASSIC_COMPILER_CONFIG_FILES = ['remix.config.js']
const CLASSIC_COMPILER_DEV = {
command: 'remix dev --manual -c "netlify dev"',
}
const CLASSIC_COMPILER_BUILD = {
command: 'remix build',
directory: 'public',
}

const VITE_CONFIG_FILES = [
'vite.config.js',
'vite.config.mjs',
'vite.config.cjs',
'vite.config.ts',
'vite.config.mts',
'vite.config.cts',
]
const VITE_DEV = {
command: 'shopify hydrogen dev',
port: 5173,
}
const VITE_BUILD = {
// This should be `shopify hydrogen build` but we use this as a workaround for
// https://github.com/Shopify/hydrogen/issues/2496 and https://github.com/Shopify/hydrogen/issues/2497.
command: 'remix vite:build',
directory: 'dist/client',
serhalp marked this conversation as resolved.
Show resolved Hide resolved
}

export class Hydrogen extends BaseFramework implements Framework {
readonly id = 'hydrogen'
name = 'Hydrogen'
npmDependencies = ['@shopify/hydrogen']
category = Category.SSG

dev = {
command: 'vite',
port: 3000,
pollingStrategies: [{ name: 'TCP' }],
}

build = {
command: 'npm run build',
directory: 'dist/client',
}

logo = {
default: '/logos/hydrogen/default.svg',
light: '/logos/hydrogen/default.svg',
dark: '/logos/hydrogen/default.svg',
}

async detect(): Promise<DetectedFramework | undefined> {
await super.detect()

if (this.detected) {
const viteDetection = await this.detectConfigFile(VITE_CONFIG_FILES)
if (viteDetection) {
this.detected = viteDetection
this.dev = VITE_DEV
this.build = VITE_BUILD
return this as DetectedFramework
}
const classicCompilerDetection = await this.detectConfigFile(CLASSIC_COMPILER_CONFIG_FILES)
if (classicCompilerDetection) {
this.detected = classicCompilerDetection
this.dev = CLASSIC_COMPILER_DEV
this.build = CLASSIC_COMPILER_BUILD
return this as DetectedFramework
}
// If neither config file exists, it can't be a valid Hydrogen site for Netlify anyway.
return
}
pieh marked this conversation as resolved.
Show resolved Hide resolved
}
}
Loading
Loading