gulpjs / liftoff

Launch your command line tool with ease.
MIT License
840 stars 52 forks source link

can't use '.'(dot) in 'configName' option #100

Closed EladBezalel closed 5 years ago

EladBezalel commented 5 years ago

When creating a new Liftoff instance and passing a configName you can't use . as part of it.

I want to have a cli named dcm and it's config file to be dcm.config.{ext}

import Liftoff from 'liftoff';
import { jsVariants } from 'interpret';

const Dcm = new Liftoff({
  name: 'dcm',
  configName: 'dcm.config',
  extensions: jsVariants
});

Dcm.prepare({}, env =>
  Dcm.execute(env, () => {
    const config = env.configPath ? require(env.configPath).default : undefined;
    // do something with config
  })
)

at this point when requiring the config the registered loaders doesn't seem to run properly, when removing the . from the name or having es5 code syntax in the config file, it works.

sttk commented 5 years ago

@EladBezalel On node --experimental-modules, the above code outputs error message: UnhandledPromiseRejectionWarning: ReferenceError: require is not defined.

How about using import() instead of require()?

EladBezalel commented 5 years ago

It doesn't seem to be related to require nor import

How is --experimental-modules related to this issue?

For me it seems as somewhere along the way some package is splitting by . and loses context of the registered loaders for that file type.

sttk commented 5 years ago

I'm sorry if I misunderstood. I thought this issue that the above code failed to run at requiring, and tried it as follows. (About --experimental-modules it was for explanation of my execution setting.)

$ cat dcm.config.js 
exports.hello = 'Hello'
$ cat dcm.mjs
import Liftoff from 'liftoff '
import interpret from 'interpret'
const Dcm = new Liftoff({
  name: 'dcm',
  configName: 'dcm.config',
  extensions: interpret.jsVariants,
})
Dcm.prepare({}, env =>
  Dcm.execute(env, async () => {
    const cfg = env.configPath ? require(env.configPath) : undefined
    console.log(cfg)
    console.log(env)
  })
)
$ node --experimental-modules dcm.mjs
(node:24388) ExperimentalWarning: The ESM module loader is experimental.
(node:24388) UnhandledPromiseRejectionWarning: ReferenceError: require is not defined
 〜
$ vi dcm.mjs
   〜
    const cfg = env.configPath ? require(env.configPath) : undefined;
    → const cfg = env.configPath ? await import(env.configPath) : undefined;
 〜
$ node --experimental-modules dcm.mjs
(node:24423) ExperimentalWarning: The ESM module loader is experimental.
[Module] { default: { hello: 'Hello' } }
{ cwd: '/path/to/issue100',
  require: [],
  configNameSearch:
   [ 'dcm.config.js',
     'dcm.config.babel.js',
     'dcm.config.babel.ts',
     'dcm.config.buble.js',
     'dcm.config.cirru',
     'dcm.config.cjsx',
     'dcm.config.co',
     'dcm.config.coffee',
     'dcm.config.coffee.md',
     'dcm.config.eg',
     'dcm.config.esm.js',
     'dcm.config.iced',
     'dcm.config.iced.md',
     'dcm.config.jsx',
     'dcm.config.litcoffee',
     'dcm.config.liticed',
     'dcm.config.ls',
     'dcm.config.ts',
     'dcm.config.tsx',
     'dcm.config.wisp' ],
  configPath: '/path/to/issue100/dcm.config.js',
  configBase: '/path/to/issue100',
  modulePath: undefined,
  modulePackage: {},
  configFiles: {} }
$

What result do you mention improper? Could you show me any output of running?

EladBezalel commented 5 years ago

I think I have forgot to mention my config file is in typescript

export default {
  a: 'a'
}

By saying that the registered loaders aren't working I meant that when I have . in the config file it doesn't transpiles the file, even with Babel It works when I remove the dot from the config name.

I hope that's clearer

sttk commented 5 years ago

@EladBezalel is this what you want? If this is different, please post your execution example.

$ node -v
v10.15.3
$ npm -v
6.4.1
$ cat dcm.js
import Liftoff from 'liftoff';
import { jsVariants } from 'interpret';
const Dcm = new Liftoff({
  name: 'dcm',
  configName: 'dcm.config',
  extensions: jsVariants
});
Dcm.prepare({}, env =>
  Dcm.execute(env, () => {
    const config = env.configPath ? require(env.configPath).default : undefined;
    console.log(config);
  })
)
$ cat dcm.config.ts 
export default {
  a: 'a'
}
$ npm i liftoff interpret ts-node babel-cli babel-preset-env
$ echo '{ "presets": ["env"] }' > .babelrc
$ npx babel-node dcm.js
{ a: 'a' }
$
EladBezalel commented 5 years ago

So your example is working, though when i try to create a cli tool out of it it seems to fail..

I replicated it here on glitch

Once i move the Liftoff preparation and execution inside a function it fails with the dot in the name..

Thanks for looking into it

sttk commented 5 years ago

@EladBezalel Thanks for your code. I could reproduce this issue.

The cause of this issue is rechoir#37 and this was already fixed but liftoff has not update it yet.

I've sent a PR and this issue will be fixed by it.

EladBezalel commented 5 years ago

That's awesome, thanks for looking into it. love your project, keep up the good work :)