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

Allow custom Drivers to be defined in the scope of the provider #144

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

Allow custom Drivers to be defined in the scope of the provider #144

niieani opened this issue Sep 5, 2021 · 4 comments

Comments

@niieani
Copy link
Contributor

niieani commented Sep 5, 2021

Hi Miles! First of all, amazing work on the package, loving the work so far - thank you! 馃檱

Since writing drivers isn't documented, after a lot of trial and error I've kinda reverse engineered how they work. I understand the limitation is that drivers need to be external packages. This is a huge pain, especially when trying out stuff.

It would be great if we could write Drivers locally scoped to the provider, the same way we can do it for Scripts, i.e. by creating a directory drivers under the provider root.

I've worked around this personally by manually loading the local driver in the bootstrap of my provider (src/index.ts):

export default async function bootstrap(tool: Tool) {
  const {vite} = tool.config.settings
  if (vite) {
    await tool.driverRegistry.load(
      require.resolve('../drivers/Vite.ts'),
      undefined,
      {tool},
    )
  }
}

Unfortunately that this means my consumer can't define the driver in the driver array like:

  drivers: ['eslint', 'jest', 'prettier', 'typescript', 'vite'],

But instead I need to provide the configuration options in the settings:

export default {
  module: '@niieani/beemo-build-tools',
  drivers: ['babel', 'eslint', 'jest', 'prettier', 'typescript'],
  settings: {
    vite: {},
  },
} as BeemoConfig

It would be great if this module-scoped driver resolution was supported out-of-the-box!

@milesj
Copy link
Collaborator

milesj commented Sep 6, 2021

@niieani I agree, but I'm not sure what the config would look like for consumers. When configuring the drivers, it must be a file system path or a node module, and I'm not sure there's another option here (the plugin system is abstracted outside of Beemo).

This is a bit janky, but may work.

export default {
  module: '@niieani/beemo-build-tools',
  drivers: ['babel', 'eslint', 'jest', 'prettier', 'typescript', './node_modules/@niieani/beemo-build-tools/drivers/Vite.js'],
  settings: {
    vite: {},
  },
} as BeemoConfig

Or even easier with require.resolve.

export default {
  module: '@niieani/beemo-build-tools',
  drivers: ['babel', 'eslint', 'jest', 'prettier', 'typescript', require.resolve('@niieani/beemo-build-tools/drivers/Vite.js')],
  settings: {
    vite: {},
  },
} as BeemoConfig

@niieani
Copy link
Contributor Author

niieani commented Sep 6, 2021

Since module is literally the property above ('@niieani/beemo-build-tools'), couldn't Beemo itself try resolving it relative to it, aside from looking at installed node_modules?

@milesj
Copy link
Collaborator

milesj commented Sep 6, 2021

@niieani I'd rather not. When fs paths are used, it's not apparent where they would resolve from. Cwd? Parent dir? Config module? Supplying absolute paths is typically the path of least resistance (require.resolve, path.join(__dirname), etc).

@niieani
Copy link
Contributor Author

niieani commented Sep 7, 2021

OK, gotcha. Thanks for the quick reply.

Aren't Scripts being loaded relative to the provider module already today? Couldn't Drivers just use the same resolution mechanisms?

resolver
.lookupFilePath(`scripts/${fileName}.ts`, root)
.lookupFilePath(`scripts/${fileName}.js`, root)
.lookupFilePath(`src/scripts/${fileName}.ts`, root)
.lookupFilePath(`lib/scripts/${fileName}.js`, root);
} else {
resolver
.lookupNodeModule(`${moduleName}/scripts/${fileName}.ts`)
.lookupNodeModule(`${moduleName}/scripts/${fileName}.js`)
.lookupNodeModule(`${moduleName}/src/scripts/${fileName}.ts`)
.lookupNodeModule(`${moduleName}/lib/scripts/${fileName}.js`)
.lookupNodeModule(tool.scriptRegistry.formatModuleName(context.scriptName, true))
.lookupNodeModule(tool.scriptRegistry.formatModuleName(context.scriptName));

If absolute path work today, then that's a fair enough workaround. Would be good to have it documented though.

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

2 participants