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

dmno build - flatten services config for use within docker (or similar) scenarios #118

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

theoephraim
Copy link
Member

@theoephraim theoephraim commented Aug 8, 2024

DMNO allows you to reuse config across a monorepo, picking config items from other services (both parents and siblings). In a scenario where we do not have access to the entire monorepo (like in a docker image), we no longer have access to the config files of these other services (the ones being picked from) or their dependencies (node_modules). So we must expose these config files and required dependencies somehow. To make this simpler for end users, we will provide a dmno build command which flattens the config and dependencies into something that can be included in the final build artifact without needing to preserve the entire monorepo.

The strategy here is to create a new .dmno-built directory which has a directory for each service (predecessors in the services DAG only). We'll perform a build (powered by rollup) on each config file, and can inline required dependencies. One complicating factor is that we do want to preserve a single version of dmno itself, and any plugins that are being injected, since we use instanceof checks in some places, and plugin authors may be making use of the module context or the plugin class for singletons. Eventually we will need to deal with warning users about version mismatches between services - but this should be doable since we have everything available at build time.

One complicating factor is that some dependencies could install additional binaries or have build artifacts from installation that would need to be made available in the child. The simple workaround is to explicitly install those dependencies in the child service as well, even if it is not explicitly used in the child's dmno config. We need to think through if there is any better way to deal with this...

This first iteration is not perfect, but it works and it seems the strategy is sound. This should be usable to test out the DX as long as users are aware of the versioning and binary caveats mentioned above.

Also note that currently, I'm naively just including the entire config, but since we understand the full graph, we should be able to remove unrelated config items in the predecessor services, which should help enable further tree-shaking of dependencies.

Copy link

changeset-bot bot commented Aug 8, 2024

🦋 Changeset detected

Latest commit: 4cae8c3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@dmno/1password-plugin Patch
dmno Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

netlify bot commented Aug 8, 2024

Deploy Preview for dmno ready!

Name Link
🔨 Latest commit 4cae8c3
🔍 Latest deploy log https://app.netlify.com/sites/dmno/deploys/66b55bbe996fd60008f70e7a
😎 Deploy Preview https://deploy-preview-118--dmno.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Aug 8, 2024

Deploy Preview for signup-api canceled.

Name Link
🔨 Latest commit 4cae8c3
🔍 Latest deploy log https://app.netlify.com/sites/signup-api/deploys/66b55bbe1d3e310008afef5c

// TODO: will need to figure out how to generalize this
// we could copy everything that is not git-ignored?
// we could let plugins specify a list of patterns to copy
await exec(`cp ${workspaceService.path}/.dmno/*.vault.json ${buildPackageDirPath}/.dmno`);
Copy link
Member Author

Choose a reason for hiding this comment

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

there are other items in the .dmno folder which may be needed after flattening - for example our vault file(s) from the encrypted vault plugin. For now this is hardcoded, but we'll either need to rely on git to know which files to copy, or let plugins define some glob pattern of files they care about.

await exec(`cp ${workspaceService.path}/.dmno/*.vault.json ${buildPackageDirPath}/.dmno`);

// copy package.json file
await copyFile(`${workspaceService.path}/package.json`, `${buildPackageDirPath}/package.json`);
Copy link
Member Author

@theoephraim theoephraim Aug 8, 2024

Choose a reason for hiding this comment

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

I'm copying the package.json files. Currently it's only used to get the package name, but we could skip this and store it elsewhere. We also probably want to remove the strict need for having a package.json file anyway, to start supporting more polyglot use-cases.

await copyFile(`${workspaceService.path}/package.json`, `${buildPackageDirPath}/package.json`);
}

await writeFile(`${buildDirPath}/dmno-build-info.json`, JSON.stringify(buildMetadata, null, 2), 'utf8');
Copy link
Member Author

Choose a reason for hiding this comment

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

Not much in this file yet (see above) but it at least gives us somewhere to put extra data.

addCacheFlags(program);
addServiceSelection(program);

program.action(async (opts: {
Copy link
Member Author

Choose a reason for hiding this comment

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

most of the logic in here should probably move somewhere else, but just wanted to get it working as a POC.

@@ -540,6 +540,13 @@ export class DmnoWorkspace {
}
throw new Error(`unable to find service - ${descriptor}`);
}
getServiceMetaForBuild(serviceName: string) {
Copy link
Member Author

Choose a reason for hiding this comment

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

we'll probably want to include more metadata here, like which plugins are required, what their dependency versions are, which config items in the related services are actually being used, etc

@@ -99,10 +99,18 @@ export class ConfigLoader {
}

async reload() {
// make sure everything is initialized
await this.isReady;
if (!this.workspaceInfo) await this.finishInit();
Copy link
Member Author

Choose a reason for hiding this comment

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

had to rework the init flow, since we have the new -b/--built flag which affects the initial load. This isn't quite right but seems ok for now.

// make sure everything is initialized
// await this.isReady;

if (!this.viteNodeRunner) {
Copy link
Member Author

Choose a reason for hiding this comment

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

moving the vite server init is definitely going to make things slower, so we should try to move it and make sure it's happening as early as possible.

Copy link

pkg-pr-new bot commented Aug 9, 2024

commit: 4cae8c3

pnpm add https://pkg.pr.new/@dmno/1password-plugin@118
pnpm add https://pkg.pr.new/dmno@118

Open in Stackblitz

checkForSchemaErrors(workspace);

if (!ctx.configLoader.workspaceInfo.isMonorepo) {
throw new CliExitError('`dmno build` only works in monorepos');
Copy link
Contributor

@philmillman philmillman Aug 9, 2024

Choose a reason for hiding this comment

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

maybe something slightly more explicit here, like "dmno build is only necessary in monorepos, please use dmno run"

Copy link
Member Author

Choose a reason for hiding this comment

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

maybe just link to docs page

@rawkode
Copy link

rawkode commented Sep 9, 2024

What's the status of this?

@theoephraim
Copy link
Member Author

@rawkode - was a working proof of concept, but I need to come back to it. Do you have the rest of your stuff in a state to test it out (one I rebase and get it working again?)

@theoephraim theoephraim added the maintainer Created by an official project maintainer label Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintainer Created by an official project maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants