Skip to content
This repository has been archived by the owner on Nov 9, 2022. It is now read-only.

New Config Strategy Proposal: STRATEGY_MAKECONFIG_REFERENCE #145

Open
niieani opened this issue Sep 5, 2021 · 17 comments
Open

New Config Strategy Proposal: STRATEGY_MAKECONFIG_REFERENCE #145

niieani opened this issue Sep 5, 2021 · 17 comments

Comments

@niieani
Copy link
Contributor

niieani commented Sep 5, 2021

One of the significant limitations of beemo today is that you can't create drivers that make configs from non-serializable configurations, e.g. that include classes/instances or functions in the configs.

I propose a new strategy to generate configs, similar to STRATEGY_REFERENCE.

Instead of it simply re-exporting the provider config:

await fs.writeFile(configPath.path(), `module.exports = require('./${requirePath}');`);

The provider reference would be required to export a makeConfig function (instead of an actual config object), that takes in 2 arguments: driver options object and the optional local config override and returns the config. The resulting generated config file would look something like this:

import {makeConfig} from '@niieani/beemo-build-tools/configs/vite.ts';
import overrides from './.config/beemo/vite.ts';
export default makeConfig(
  {
    "some-custom-option": "123"
  },
  overrides
);

Since some configs don't understand TypeScript, it might be necessary for both the consumer overrides and the provider's makeConfig were written in pure JS, so the emit would be:

const {makeConfig} = require('@niieani/beemo-build-tools/configs/vite.js');
module.exports = makeConfig(
  {
    "some-custom-option": "123"
  },
  require('./.config/beemo/vite.js')
);

I've hacked this together today using the template strategy (nice flexibility!), but it would be nice to have something built-in as a shorthand for this:

export default function customTemplate(
  [providerConfig, consumerConfig]: ConfigObject[],
  options: ConfigTemplateOptions,
): ConfigTemplateResult {
  const {configModule} = options
  const providerConfigModule = `${configModule}${options.providerConfigPath
    .path()
    .split(configModule)
    .pop()}`

  const consumerPath =
    consumerConfig &&
    options.tool.project.root.relativeTo(options.consumerConfigPath).path()

  const {
    args,
    configStrategy,
    dependencies,
    env,
    expandGlobs,
    outputStrategy,
    template,
    // omit internals, keep only actual config:
    ...driverOptions
  } = (options.driver as ViteDriver).options

  return {
    config: [
      `import {makeConfig} from '${providerConfigModule}';`,
      consumerPath ? `import overrides from './${consumerPath}';` : undefined,
      `export default makeConfig(`,
      `  ${JSON.stringify(driverOptions, null, 2).split('\n').join('\n  ')},`,
      consumerPath ? '  overrides' : undefined,
      `);`,
    ]
      .filter(Boolean)
      .join('\n'),
  }
}
@milesj
Copy link
Collaborator

milesj commented Sep 6, 2021

@niieani I've thought about this quite a bit, but the problem always came down to individual drivers. The implementation for this would be different between each driver, and most underlying tools don't provide an API for "merging" multiple configs.

But having the consumer provide the merge API may be viable.

@niieani
Copy link
Contributor Author

niieani commented Sep 6, 2021

But having the consumer provide the merge API may be viable.

Did you mean the provider - i.e. consumer configures overrides as it pleases, and provider handles the merging?
Doing it in reverse might work too, but then every consumer has to reimplement the merging, whereas when the provider handles merging, it's only implemented once.

@milesj
Copy link
Collaborator

milesj commented Sep 6, 2021

Consumer is the one the configures ".config/beemo.ts", which is where the strategy would be configured. So it would fall on them to implement this, but the provider can provide the merge utility for convenience.

@niieani
Copy link
Contributor Author

niieani commented Sep 7, 2021

I see. My thinking is this could also be set as the default strategy by the driver?
This way the flow is IMO most transparent:

  • the provider's job is to override defaults set by a given tool (e.g. webpack)
  • consumer may choose to add additional config if they wish, but they don't have to configure anything, just provide their config overrides in the standard format that the tool expects (e.g. standard webpack config)
  • provider generates a config that, during tool's runtime/initialization, merges the optional config provided by the consumer, with it's own config (see example above)

This way nothing changes from the consumer's perspective, they still provide a standard config for the tool, but are able to use non-serializable values, because config isn't merged statically, but in runtime.

@milesj
Copy link
Collaborator

milesj commented Sep 7, 2021

@niieani Yeah that's possible if the provider configures the driver through events.

@hayes
Copy link
Contributor

hayes commented Sep 7, 2021

This can also be implemented with the template strategy. A provider can create a default template that basically imports the user config, the provider config, and the merge function and exports the merged config

@niieani
Copy link
Contributor Author

niieani commented Sep 7, 2021

@hayes not sure if you read my first comment fully, but that's exactly what I did. But since it's a workaround, it needs re-implementing of the same logic every time you'd want to do this, hence the proposal is to make a strategy like that built-in.

Also, provider cannot set the template strategy as the default strategy. Only the consumer can use the template strategy. Perhaps that limitation could also be lifted. Right now I have to workaround this by overriding the config provided by the consumer:

export default function viteDriver(options?: ViteDriverOptions) {
  return new ViteDriver({
    configStrategy: 'template',
    template: require.resolve('./template.ts'),
    ...options,
  })
}

@milesj
Copy link
Collaborator

milesj commented Sep 7, 2021

@niieani The provider can mutate drivers using events, like so: https://github.com/beemojs/dev/blob/master/packages/dev/src/index.ts#L73

@niieani
Copy link
Contributor Author

niieani commented Sep 8, 2021

Good to know!

Anyhow, do you think it would be valuable to create something built-in to deal with this @milesj?

I can take a stab at a PR, just want to know your opinion/thoughts/guidelines so my work doesn't go to waste.

@hayes
Copy link
Contributor

hayes commented Sep 8, 2021

I had looked into this a little. The biggest issue I ran into was that there isn't a great way to access the driver specific merge functions at the time when the config files are executed. They are executed by the tool, so beemo can't inject beemo or driver instance, and creating instances of the driver, or accessing merge directly through the prototype seemed a little hacky.

Using events to configure template as the default strategy with the right templates worked pretty well and didn't require anything specific in the consumer. Not ideal, but implementing this in beemo itself would require a consistent way to get merge functions from a driver outside the scope of a beemo process since config files need to work even when the tool is not invoked directly though beemo

@niieani
Copy link
Contributor Author

niieani commented Sep 9, 2021

The biggest issue I ran into was that there isn't a great way to access the driver specific merge functions at the time when the config files are executed.

Am I right to assume that the reason for this is that beemo is initializing asynchronously, and we can't simply require it inside of the config file, since not all config files allow asynchronous resolution? There might be a way around that by using deasync. First line of generated config could bootstrap beemo and make it available to the context of the config file, before yielding the config file to the tool that required it.

Alternatively, this could work only for tools that allow returning promises as config. Surprisingly, a lot of tools allow this (e.g. both webpack and vite handle returning promises from the config). Would be fairly easy to bootstrap beemo for those.

@milesj
Copy link
Collaborator

milesj commented Sep 9, 2021

There's 2 phases for config files, generation and evaluation (execution).

Beemo only handles generation because we control the merging and creation of the config files. At this point, the Beemo tool/driver are available under process.beemo.

The evaluation phase happens outside of Beemo, since its the third-party tools (Babel, Jest, etc) running their own process, loading the config files on their own, and doing what they need to do. Beemo has no context into this process, so the current tool/driver are not available.

@niieani
Copy link
Contributor Author

niieani commented Sep 9, 2021

I understand this @milesj, my question is about the reason why we couldn't add Beemo into the evaluation phase? It's initiated outside of Beemo, but is there a reason why we can't initiate Beemo inside of it?

@milesj
Copy link
Collaborator

milesj commented Sep 9, 2021

@niieani What do you mean by initiate exactly? There's many layers to Beemo.

@niieani
Copy link
Contributor Author

niieani commented Sep 10, 2021

@milesj by initiate I mean make process.beemo available for the tool, complete instance and populated with the configuration from the .config directory

@milesj
Copy link
Collaborator

milesj commented Sep 10, 2021

I'm not sure of a way to modify the process global of another process. That seems like a security risk.

@niieani
Copy link
Contributor Author

niieani commented Sep 10, 2021

Oh, nothing as fancy as that. I wouldn't even know if that's possible without corrupting the process 😅

I see two ways this could be done, both assume the tool that requires the config is node-based, and that the config itself is a JavaScript file:

  • wrap node itself, so that when running say beemo exec webpack it will execute webpack in the same process (instead of spawning a subprocess), but first it would setup process.beemo in it. Limitation: only works for CLI tools, so e.g. babel, which can be used from within other processes, can't be used this way.
  • execute a pipeline similar to RunDriverPipeline inside of the config file; if the tools supports async config return, then it's easy, otherwise synchronously with deasync in the first line of the config

But even without implementing either option, so without having access to process.beemo from within the config, I think the merging by reference can be done, because we can statically generate the config, assuming given Driver's beemo.settings are JSON serializable. That's basically what I've done with my template in the first post here.

If you think this is too far fetched to be built-in, maybe you'd be open to having pluggable config strategies, so this could be implemented outside of the project? E.g. beemo-config-strategy-my-custom-strategy?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants