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

Resolve incosistency in directory option in JS vs CLI usage #355

Closed
Tracked by #365
webketje opened this issue Dec 19, 2021 · 3 comments
Closed
Tracked by #365

Resolve incosistency in directory option in JS vs CLI usage #355

webketje opened this issue Dec 19, 2021 · 3 comments
Milestone

Comments

@webketje
Copy link
Member

webketje commented Dec 19, 2021

There is an inconsistency in the metalsmith CLI vs JS approach: in the CLI Metalsmith.directory is defaulted to process.cwd(). In the JS approach there is no default and __dirname needs to be passed explicitly. Solution:A) force specifying a directory option in metalsmith.json (breaking) or B) Make __dirname also the default for the JS solution (backwards-compatible)

@webketje webketje mentioned this issue Dec 19, 2021
12 tasks
@webketje webketje changed the title There is an inconsistency in the metalsmith CLI vs JS approach: in the CLI Metalsmith.directory is defaulted to process.cwd(). In the JS approach there is no default and __dirname needs to be passed explicitly. Solution:A) force specifying a directory option in metalsmith.json (breaking) or B) Make __dirname also the default for the JS solution (backwards-compatible) Resolve incosistency in directory option in JS vs CLI usage Dec 19, 2021
@webketje
Copy link
Member Author

webketje commented Jan 2, 2022

To resolve the inconsistency, path.dirname(path) of the metalsmith.json config path must be used as the metalsmith.directory. This would allow the CLI to properly execute metalsmith from a subfolder of a project/package, as the JS approach already does

@webketje
Copy link
Member Author

webketje commented Jan 9, 2022

See https://replit.com/@webketje/Metalsmith-CLI-dirpath-tests which does multiple CLI tests from different $PWD paths. Below is a test run. The tests that start with "Next - " use a modified path resolution for local plugins & loading the config that is a candidate for Metalsmith 2.4+

[
  {
    chdir: '/home/runner/Metalsmith-CLI-dirpath-tests',
    name: 'Repo root (default)',
    success: true
  },
  {
    chdir: '/home/runner/Metalsmith-CLI-dirpath-tests',
    name: 'Next - Repo root (default)',
    success: true
  },
  {
    chdir: '/home/runner/Metalsmith-CLI-dirpath-tests',
    name: 'Repo root (absolute config path)',
    success: true
  },
  {
    chdir: '/home/runner/Metalsmith-CLI-dirpath-tests',
    name: 'Next - Repo root (absolute config path)',
    success: true
  },
  {
    chdir: '/home/runner/Metalsmith-CLI-dirpath-tests/src',
    name: 'Repo subdir',
    error: 'Command failed: node ../bin/metalsmith -c ../metalsmith.json'
  },
  {
    chdir: '/home/runner/Metalsmith-CLI-dirpath-tests/src',
    name: 'Next - Repo subdir',
    success: true
  },
  {
    chdir: '/home/runner/Metalsmith-CLI-dirpath-tests',
    name: 'Repo subdir (absolute config path)',
    success: true
  },
  {
    chdir: '/home/runner/Metalsmith-CLI-dirpath-tests',
    name: 'Next - Repo subdir (absolute config path)',
    success: true
  },
  {
    chdir: '/home/runner',
    name: 'Outside repo',
    error: 'Command failed: node ./Metalsmith-CLI-dirpath-tests/bin/metalsmith -c ./Metalsmith-CLI-dirpath-tests/metalsmith.json'
  },
  { chdir: '/home/runner', name: 'Next - Outside repo', success: true }
]

As you can see with Metalsmith 2.3, running the metalsmith bin from a path other than the repo root fails. It fails because it resolves local plugins and Metalsmith.directory to process.cwd()

@webketje
Copy link
Member Author

webketje commented Jan 9, 2022

It looks interesting to do some default path resolution in both the CLI as the JS API, especially given the unavailability of __dirname in ES modules, see https://techsparx.com/nodejs/esnext/dirname-es-modules.html

@webketje webketje mentioned this issue May 23, 2022
8 tasks
@webketje webketje added this to the v2.5 milestone Jun 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant