Skip to content

Commit

Permalink
fix(dev/plugin)!: redo reverse proxy setup (#376)
Browse files Browse the repository at this point in the history
Prior to this change, assets were imported relatively, also taking into
account relativePathToPrefix. While this generally worked, there were
other issues due to Vite's build optimizations: specifically module
preloading and inline assets. Due to this we decided to take a new
approach where assets are now imported absolutely and it's now up to the
RP config to handle the assets. A working example of a Netlify setup for
a RP at the `subdirectory` subfolder looks like this:

```
[[redirects]]
  from = "/subdirectory/assets/*"
  to = "https://rp-test.pagesprod.yextengtest.com/subdirectory/assets/:splat"
  status = 200
  force = true
  headers = {Host = "https://rp-test.pagesprod.yextengtest.com/"}
  
[[redirects]]
  from = "/subdirectory/*"
  to = "https://rp-test.pagesprod.yextengtest.com/:splat"
  status = 200
  force = true
  headers = {Host = "https://rp-test.pagesprod.yextengtest.com/"}
  
[[redirects]]
  from = "/subdirectory"
  to = "https://rp-test.pagesprod.yextengtest.com/index.html"
  status = 200
  force = true
  headers = {Host = "https://rp-test.pagesprod.yextengtest.com/"}
```

In order to properly configure the Vite side, a user must set the
assetsDir in the vite.config.js (`subdirectory/assets` for the above
example) or in the config.yaml (which will be globally supported in some
upcoming PRs).

Other changes in the PR include a massive refactor of ProjectStructure
in order to simplify its usage and is now being used everywhere, meaning
that all filenames and folders are fully configurable.

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
mkilpatrick and github-actions[bot] authored Aug 18, 2023
1 parent 4cf67c9 commit a640802
Show file tree
Hide file tree
Showing 45 changed files with 851 additions and 496 deletions.
30 changes: 30 additions & 0 deletions packages/pages/THIRD-PARTY-NOTICES
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,36 @@ THE SOFTWARE.

-----------

The following npm package may be included in this product:

- [email protected]

This package contains the following license and notice below:

(The MIT License)

Copyright (C) 2011-2015 by Vitaly Puzrin

Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
in the Software without restriction, including without limitation the rights
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
copies of the Software, and to permit persons to whom the Software is
furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in
all copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
THE SOFTWARE.

-----------

The following npm package may be included in this product:

- [email protected]
Expand Down
2 changes: 2 additions & 0 deletions packages/pages/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
"get-port": "^7.0.0",
"glob": "^10.3.3",
"handlebars": "^4.7.8",
"js-yaml": "^4.1.0",
"lodash": "^4.17.21",
"mime-types": "^2.1.35",
"module-from-string": "^3.3.0",
Expand All @@ -89,6 +90,7 @@
"@types/fs-extra": "^11.0.1",
"@types/glob": "^8.1.0",
"@types/jest": "^29.5.3",
"@types/js-yaml": "4.0.5",
"@types/lodash": "^4.14.197",
"@types/mime-types": "^2.1.1",
"@types/minimatch": "^5.1.2",
Expand Down
7 changes: 2 additions & 5 deletions packages/pages/src/build/build.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
import { Command } from "commander";
import { build } from "vite";
import { ProjectFilepaths } from "../common/src/project/structure.js";

type BuildArgs = Pick<ProjectFilepaths, "scope">;

const handler = async ({ scope }: BuildArgs) => {
const handler = async ({ scope }: { scope: string }) => {
// Pass CLI arguments as env variables to use in vite-plugin
if (scope) {
process.env.YEXT_PAGES_SCOPE = scope;
}
build();
await build();
};

export const buildCommand = (program: Command) => {
Expand Down
42 changes: 42 additions & 0 deletions packages/pages/src/common/src/assets/getAssetsFilepath.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import { determineAssetsFilepath } from "./getAssetsFilepath.js";
import * as importHelper from "./import.js";

describe("getAssetsFilepath - determineAssetsFilepath", () => {
it("returns assets when no files defined", async () => {
const actual = await determineAssetsFilepath("assets", "", "");

expect(actual).toEqual("assets");
});

it("returns custom assetsDir from config.yaml", async () => {
const actual = await determineAssetsFilepath(
"assets",
"tests/fixtures/config.yaml",
"tests/fixtures/vite.config.js"
);

expect(actual).toEqual("subpath/assets");
});

it("returns custom assets from vite config when no config.yaml", async () => {
const viteConfig = {
default: {
plugins: [],
build: {
assetsDir: "viteConfigAssetsDir",
},
},
};

const importSpy = jest.spyOn(importHelper, "import_");
importSpy.mockImplementation(async () => viteConfig);

const actual = await determineAssetsFilepath(
"assets",
"tests/fixtures/invalid.yaml",
"does not matter since mocked"
);

expect(actual).toEqual("viteConfigAssetsDir");
});
});
39 changes: 39 additions & 0 deletions packages/pages/src/common/src/assets/getAssetsFilepath.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import fs from "node:fs";
import { pathToFileURL } from "url";
import { UserConfig } from "vite";
import { import_ } from "./import.js";
import yaml from "js-yaml";
import { ConfigYaml } from "../config/config.js";

/**
* Determines the assets directory to use by checking the following, in order:
* 1. config.yaml
* 2. vite.config.json assetDir
* 3. default to "assets"
* @param servingJsonPath the path to sites-config/serving.json - make sure to take scope into account
* @param viteConfigPath the path to vite.config.js
*/
export const determineAssetsFilepath = async (
defaultAssetsDir: string,
configYamlPath: string,
viteConfigPath: string
) => {
if (configYamlPath === "" || viteConfigPath === "") {
return defaultAssetsDir;
}

if (fs.existsSync(configYamlPath)) {
const configYaml = yaml.load(
fs.readFileSync(configYamlPath, "utf-8")
) as ConfigYaml;

if (configYaml.assetsDir && configYaml.assetsDir !== "") {
return configYaml.assetsDir;
}
}

const viteConfig = await import_(pathToFileURL(viteConfigPath).toString());
const userConfig = viteConfig.default as UserConfig;

return userConfig.build?.assetsDir ?? defaultAssetsDir;
};
6 changes: 6 additions & 0 deletions packages/pages/src/common/src/assets/import.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/**
* A custom import function so that it can be mocked in tests.
*/
export const import_ = async (filepath: string) => {
return await import(filepath);
};
7 changes: 7 additions & 0 deletions packages/pages/src/common/src/config/config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export interface ConfigYaml {
/**
* The folder path that assets will be served from. If your using a reverse proxy at
* a subpath, this should typically look like "mySubpath/assets".
* */
assetsDir?: string;
}
32 changes: 25 additions & 7 deletions packages/pages/src/common/src/feature/features.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,22 @@ import {
FeaturesConfig,
} from "./features.js";
import { TemplateConfigInternal } from "../template/internal/types.js";
import { ProjectStructure } from "../project/structure.js";

describe("features - convertTemplateConfigToFeaturesConfig", () => {
const projectStructure = new ProjectStructure();

it("returns a FeaturesConfig with no StreamConfig if no stream is defined", async () => {
const templateConfig: TemplateConfigInternal = {
name: "myTemplateConfig",
streamId: "$id",
hydrate: true,
};

const featuresConfig =
convertTemplateConfigInternalToFeaturesConfig(templateConfig);
const featuresConfig = convertTemplateConfigInternalToFeaturesConfig(
templateConfig,
projectStructure
);

const expected: FeaturesConfig = {
features: [
Expand Down Expand Up @@ -45,8 +50,10 @@ describe("features - convertTemplateConfigToFeaturesConfig", () => {
},
};

const featuresConfig =
convertTemplateConfigInternalToFeaturesConfig(templateConfig);
const featuresConfig = convertTemplateConfigInternalToFeaturesConfig(
templateConfig,
projectStructure
);

const expected: FeaturesConfig = {
features: [
Expand Down Expand Up @@ -76,14 +83,19 @@ describe("features - convertTemplateConfigToFeaturesConfig", () => {
});

describe("features - convertTemplateConfigToFeatureConfig", () => {
const projectStructure = new ProjectStructure();

it("uses the streamId if defined and return an EntityPageSetConfig", async () => {
const templateConfig: TemplateConfigInternal = {
name: "myTemplateConfig",
hydrate: true,
streamId: "$id",
};

const featureConfig = convertTemplateConfigToFeatureConfig(templateConfig);
const featureConfig = convertTemplateConfigToFeatureConfig(
templateConfig,
projectStructure
);

const expected: FeatureConfig = {
name: "myTemplateConfig",
Expand All @@ -109,7 +121,10 @@ describe("features - convertTemplateConfigToFeatureConfig", () => {
},
};

const featureConfig = convertTemplateConfigToFeatureConfig(templateConfig);
const featureConfig = convertTemplateConfigToFeatureConfig(
templateConfig,
projectStructure
);

const expected: FeatureConfig = {
name: "myTemplateConfig",
Expand All @@ -126,7 +141,10 @@ describe("features - convertTemplateConfigToFeatureConfig", () => {
name: "myTemplateConfig",
hydrate: true,
};
const featureConfig = convertTemplateConfigToFeatureConfig(templateConfig);
const featureConfig = convertTemplateConfigToFeatureConfig(
templateConfig,
projectStructure
);

const expected: FeatureConfig = {
name: "myTemplateConfig",
Expand Down
21 changes: 15 additions & 6 deletions packages/pages/src/common/src/feature/features.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import fs from "fs";
import path from "path";
import { TemplateConfigInternal } from "../template/internal/types.js";
import { convertTemplateConfigToStreamConfig, StreamConfig } from "./stream.js";
import { unsecureHashPluginName } from "../function/internal/types.js";
import { defaultProjectStructureConfig } from "../project/structure.js";
import { ProjectStructure } from "../project/structure.js";

/**
* The shape of data that represents a features.json file, used by Yext Pages.
Expand All @@ -18,9 +19,13 @@ export interface FeaturesConfig {
* Converts a {@link TemplateConfigInternal} into a valid {@link FeaturesConfig} (features and streams).
*/
export const convertTemplateConfigInternalToFeaturesConfig = (
config: TemplateConfigInternal
config: TemplateConfigInternal,
projectStructure: ProjectStructure
): FeaturesConfig => {
const featureConfig = convertTemplateConfigToFeatureConfig(config);
const featureConfig = convertTemplateConfigToFeatureConfig(
config,
projectStructure
);
const streamConfig = convertTemplateConfigToStreamConfig(config);

return {
Expand Down Expand Up @@ -72,7 +77,8 @@ export const isStaticTemplateConfig = (
* Converts a {@link TemplateConfigInternal} into a valid single {@link FeatureConfig}.
*/
export const convertTemplateConfigToFeatureConfig = (
config: TemplateConfigInternal
config: TemplateConfigInternal,
projectStructure: ProjectStructure
): FeatureConfig => {
const streamConfig = config.stream || null;

Expand All @@ -89,8 +95,11 @@ export const convertTemplateConfigToFeatureConfig = (
try {
// Have to look up the filename from the functions directory because we do not know file extension
const onUrlChangeFilenames = fs.readdirSync(
defaultProjectStructureConfig.filepathsConfig.functionsRoot +
"/onUrlChange"
path.join(
projectStructure.config.rootFolders.source,
projectStructure.config.subfolders.serverlessFunctions,
"onUrlChange"
)
);
const filename = onUrlChangeFilenames.find((name) =>
name.includes(config.onUrlChange ?? "")
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
import path from "path";
import { getFunctionFilepaths } from "./getFunctionFilepaths.js";
import { minimatch } from "minimatch";
import { defaultProjectStructureConfig } from "../../project/structure.js";

const rootPath = defaultProjectStructureConfig.filepathsConfig.functionsRoot;
const multiLevelPath =
defaultProjectStructureConfig.filepathsConfig.functionsRoot +
"/http/api/fetch";
const rootPath = "src/functions";
const multiLevelPath = "src/functions/http/api/fetch";

const filepaths = [
`${multiLevelPath}/test1.ts`,
Expand Down
15 changes: 13 additions & 2 deletions packages/pages/src/common/src/function/internal/loader.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import path from "path";
import { loadFunctionModules, FunctionModuleCollection } from "./loader.js";
import { ProjectStructure } from "../../project/structure.js";

// our jest configuration doesn't support file urls so update pathToFileURL to do nothing during
// this test.
Expand All @@ -21,20 +22,30 @@ jest.mock("vite", () => {
afterAll(() => jest.unmock("url"));

describe("loadTemplateModules", () => {
const projectStructure = new ProjectStructure();

it("loads and transpiles raw templates", async () => {
const functionFile: path.ParsedPath[] = [
path.parse("tests/fixtures/src/functions/http/[param].ts"),
];

const functionModules = await loadFunctionModules(functionFile, true);
const functionModules = await loadFunctionModules(
functionFile,
true,
projectStructure
);
commonTests(functionModules, "param-47543");
});

it("loads transpiled templates", async () => {
const functionFile: path.ParsedPath[] = [
path.parse(path.resolve("tests/fixtures/src/functions/http/[param].js")),
];
const functionModules = await loadFunctionModules(functionFile, false);
const functionModules = await loadFunctionModules(
functionFile,
false,
projectStructure
);
commonTests(functionModules, "param-47853");
});

Expand Down
14 changes: 10 additions & 4 deletions packages/pages/src/common/src/function/internal/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { importFromString } from "module-from-string";
import { pathToFileURL } from "url";
import { getFunctionFilepaths } from "./getFunctionFilepaths.js";
import { processEnvVariables } from "../../../../util/processEnvVariables.js";
import { ProjectStructure } from "../../project/structure.js";

const TEMP_DIR = ".temp";

Expand All @@ -21,7 +22,8 @@ const TEMP_DIR = ".temp";
*/
export const loadFunctionModules = async (
functionPaths: path.ParsedPath[],
transpile: boolean
transpile: boolean,
projectStructure: ProjectStructure
): Promise<FunctionModuleCollection> => {
const importedModules = [] as FunctionModuleInternal[];
for (const functionPath of functionPaths) {
Expand Down Expand Up @@ -55,7 +57,8 @@ export const loadFunctionModules = async (
const functionModuleInternal =
convertFunctionModuleToFunctionModuleInternal(
functionPath,
functionModule
functionModule,
projectStructure
);

importedModules.push({
Expand Down Expand Up @@ -89,7 +92,10 @@ export type FunctionModuleCollection = Map<string, FunctionModuleInternal>;
* @param root the directory to check for functions
* @return Promise<FunctionModuleCollection>
*/
export const loadFunctions = async (root: string) => {
export const loadFunctions = async (
root: string,
projectStructure: ProjectStructure
) => {
const functionFilepaths = getFunctionFilepaths(root);
return await loadFunctionModules(functionFilepaths, true);
return await loadFunctionModules(functionFilepaths, true, projectStructure);
};
Loading

0 comments on commit a640802

Please sign in to comment.