bubble-dev / _

🍱 metarepo of many packages and various monorepos
52 stars 6 forks source link

🌱 start: add support for namespaces #238

Closed vonagam closed 4 years ago

vonagam commented 4 years ago

Used dot as a delimiter, but maybe it should be colon instead (similar to other task managers).

codecov[bot] commented 4 years ago

Codecov Report

Merging #238 into master will increase coverage by 0.03%. The diff coverage is 100%.

Impacted Files Coverage Δ
packages/start/cli/src/lib.ts 100% <100%> (ø) :arrow_up:
deepsweet commented 4 years ago

Hi and thanks for the contribution!

I understand this technically, but what's the rationale of namespaces from user perspective?

yarn start buildThat and yarn start build.that is kinda potato-potato as for me.

I'd personally like to keep it as simple as possible without adding more complexity dimensions, like "task vs namespace", but I might miss some use cases important for other users. "One export === one task" sounds perfect (so far).

vonagam commented 4 years ago

My problem is inability to register tasks dynamically. What i mean, imagine that you have a package that exports a bunch of plugin functions. For instance a package that provides functions to work with databases: db:create, db:drop, db:migration, db:migrate, db:seed, db:schema:dump, db:schema:load and so on, like rails ones. They all accept a database config and return a plugin function. Having config i can map an object of plugin constructors to an object of tasks, but then there is no way to register them...

How is it done now:

import * as db from '@my/start-plugin-db'

const dbConfig = { ... }

export const create = () => db.create( dbConfig )
export const drop = () => db.drop( dbConfig )
export const migrate = () => db.migrate( dbConfig )
...

What i want to have an ability to do:

import * as db from '@my/start-plugin-db'

const dbConfig = { ... }

const tasks = mapValues(db, (pluginConstructor) => () => pluginConstructor(dbConfig))

// here i need to register tasks somehow...

// if namespaces were supported, that's one way to do it:
export { tasks }

// another possibility is to allow default export to be a tasks object
export default tasks

Basically i want some way to register a task besides a named export since you cannot add a named export by code itself...

Hm... while writing it, i remembered that you can hack into exports by using module.exports, so it is actually might be possible right now (Object.assign(module.exports, tasks)). But there might be a problem with using this approach in a preset since typescript will not see those tasks in exports.

I am totally okay with you closing this PR if you think that added complexity to the library does not justify gained flexibility of tasks definitions.

deepsweet commented 4 years ago

I see. Actually we have the same setup for one of the tasks internally, and we ended up using the following approach:

export const foo = (subCommand: string) => {
  if (subCommand === 'bar') {
     return ...
  }

  if (subCommand === 'baz') {
     return ...
  }
}
yarn start foo bar
yarn start foo baz

Does this approach fit your needs as well?

Also, because literally everything in Start is "pluggable" we've been using a custom CLI instead of a generic @start/cli to "hardcode" even a Babel config and store it inside of CLI itself. Technically speaking you'd have @my/start-cli with namespaces or any other feature you'd like to have.

What do you think? Is it fair enough to close the PR?

vonagam commented 4 years ago

A function wrapper approach has a small drawback too, but it doesn't matter because your second point about start being pluggable and that default cli is really small is true. Even in this meta package you use non-default cli located at packages/bubble-dev/start-preset/src/cli.

Thanks.