-
Notifications
You must be signed in to change notification settings - Fork 58
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(build-info): fix detection of Remix and Hydrogen sites (#5820)
We noticed recently that the framework detection wasn't working correctly for all Remix sites. This leads to poor DX but it also affects our reporting that informs framework usage on the platform.
This has gotten pretty complex:
- Remix sites can either use Vite or the Remix Classic Compiler
- These use different config files, different dev and build config, but almost identical npm
dependencies (plus `vite` in the Remix Vite case)
- Hydrogen (v2) uses Remix and, as of a few months ago, it supports Remix Vite in addition to the
Remix Classic Compiler (it only supported this previously)
- Netlify's Remix packages were renamed in late 2023, but the Remix detection code only listed the
old names
- Luckily, this still happened to work because it also listed the right config file, but only for
Remix Classic Compiler sites
- However this means that in the Remix Vite case we were failing to detect Remix. Sites were
categorized as Vite instead.
- The `remix` package itself was renamed to `@remix-run/dev`, `@remix-run/serve`, etc. in 2023. We
hadn't updated this here either, which exarcerbated the above issues.
You can't actually call `test()` within a `test.each` callback. I was only doing this because I
can't use `test.for` to access the test context because we're on an old version of vitest... but
anyway, a manual table test works here.- Loading branch information
Showing
6 changed files
with
502 additions
and
29 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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) | ||
| }) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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', | ||
| } | ||
|
|
||
| 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 | ||
| } | ||
| } | ||
| } |
Oops, something went wrong.