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

Jump to bottom

copy types when using LOCAL_BUILD_DIRECTORY #8646

Draft
wants to merge 2 commits into
base: dev
Choose a base branch
from
Draft

copy types when using LOCAL_BUILD_DIRECTORY #8646

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8b1ad8d
copy types when using LOCAL_BUILD_DIRECTORY
pcattori Jan 31, 2024
e4f936a
fix permissions for remix bin
pcattori Jan 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
50 changes: 14 additions & 36 deletions rollup.utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,6 @@ const path = require("node:path");

const REPO_ROOT_DIR = __dirname;

let activeOutputDir = "build";
if (process.env.LOCAL_BUILD_DIRECTORY) {
let appDir = path.resolve(process.env.LOCAL_BUILD_DIRECTORY);
try {
fse.readdirSync(path.join(appDir, "node_modules"));
} catch {
console.error(
"Oops! You pointed LOCAL_BUILD_DIRECTORY to a directory that " +
"does not have a node_modules/ folder. Please `npm install` in that " +
"directory and try again."
);
process.exit(1);
}
console.log("Writing rollup output to", appDir);
activeOutputDir = appDir;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we lose the yarn watch functionality if we do this since it calls rollup directly. There's a watch.onEnd script we could add the copy to though if we go this route


/**
* @param {string} packageName
* @param {string} version
Expand Down Expand Up @@ -103,25 +86,20 @@ function copyToPlaygrounds() {
return {
name: "copy-to-remix-playground",
async writeBundle(options, bundle) {
if (activeOutputDir === "build") {
let playgroundsDir = path.join(REPO_ROOT_DIR, "playground");
let playgrounds = await fs.promises.readdir(playgroundsDir);
let writtenDir = path.join(REPO_ROOT_DIR, options.dir);
for (let playground of playgrounds) {
let playgroundDir = path.join(playgroundsDir, playground);
if (!fse.statSync(playgroundDir).isDirectory()) {
continue;
}
let destDir = writtenDir.replace(
path.join(REPO_ROOT_DIR, "build"),
playgroundDir
);
fse.copySync(writtenDir, destDir);
await triggerLiveReload(playgroundDir);
let playgroundsDir = path.join(REPO_ROOT_DIR, "playground");
let playgrounds = await fs.promises.readdir(playgroundsDir);
let writtenDir = path.join(REPO_ROOT_DIR, options.dir);
for (let playground of playgrounds) {
let playgroundDir = path.join(playgroundsDir, playground);
if (!fse.statSync(playgroundDir).isDirectory()) {
continue;
}
} else {
// Otherwise, trigger live reload on our LOCAL_BUILD_DIRECTORY folder
await triggerLiveReload(activeOutputDir);
let destDir = writtenDir.replace(
path.join(REPO_ROOT_DIR, "build"),
playgroundDir
);
fse.copySync(writtenDir, destDir);
await triggerLiveReload(playgroundDir);
}
},
};
Expand Down Expand Up @@ -208,7 +186,7 @@ function getCliConfig({ packageName, version }) {
* @param {string} packageName
*/
function getOutputDir(packageName) {
return path.join(activeOutputDir, "node_modules", packageName);
return path.join("build", "node_modules", packageName);
}

/**
Expand Down
20 changes: 20 additions & 0 deletions scripts/build.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import path from "node:path";
import fse from "fs-extra";
import { spawn } from "cross-spawn";

const args = process.argv.slice(2);
Expand All @@ -6,6 +8,24 @@ const tsc = process.env.CI || args.includes("--tsc") || publish;

exec("yarn", ["rollup", "-c"])
.then(() => tsc && exec("yarn", ["tsc", "-b"]))
.then(() => {
let { LOCAL_BUILD_DIRECTORY } = process.env;
if (LOCAL_BUILD_DIRECTORY) {
let appDir = path.resolve(LOCAL_BUILD_DIRECTORY);
try {
fse.readdirSync(path.join(appDir, "node_modules"));
} catch {
console.error(
"Oops! You pointed LOCAL_BUILD_DIRECTORY to a directory that " +
"does not have a node_modules/ folder. Please `npm install` in that " +
"directory and try again."
);
process.exit(1);
}
fse.copySync("build", appDir, { overwrite: true });
fse.chmodSync(path.join(appDir, "node_modules/.bin/remix"), 0o755);
}
})
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we lose the triggerLiveReload behavior if we just cp -r here? I believe we need to update one of the app files to trigger it, I don't think copying new files into node_modules as enough IIRC?

Should we be thinking about the vite cache here? Or should we just be using --force in our apps?

.then(() => publish && exec("node", ["scripts/copy-build-to-dist.mjs"]))
.then(() => process.exit(0))
.catch((err) => {
Expand Down