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

Feat: Improved DX #16

Merged
merged 8 commits into from
Sep 25, 2023
Merged

Feat: Improved DX #16

merged 8 commits into from
Sep 25, 2023

Conversation

kon-pas
Copy link
Contributor

@kon-pas kon-pas commented Sep 24, 2023

NOTE
Creating a PR to master, a branch for development-only would be awesome!

Features

Improved DX to match scaffolding tools standards.

Templates can now be created via one of the following methods:

  • Execute the package [1]
npx create-nue
bunx create-nue
  • Shorthand for executing the create-* packages [1]
npm create nue
bun create nue
yarn create nue
pnpm create nue
  • Use package entry point binary [2] (probably not preferred, but possible)
nue-create
  • Use nue CLI directly [2] (also probably not preferred, but very hand to call internally)
nue create

nue <script> runs one of the provided scripts (in /scripts as an exported funtion, see the changes below). Can also call nue render, etc.

  • Imperatively by calling the package function
import create from 'create-nue'
create( /* pass options */ )

[1] - the package has to be published
[2] - the package has to be installed locally

All of the above scaffold a placeholder, instead of a real template, because the original one is not suitable. This is a topic for another PR, which I can do once this PR gets accepted.

NOTE
It fixes the issue for Windows users, which was caused because Windows can't run a script with the './`. All of the above use package binaries. We provide a path and the package manager executes the corresponding file internally. The issue is number 8, but I don't mark it, to not close it - we scaffold only a placeholder with the above.

NOTE
To test the package by hand without publishing, it can be linked instead (run npm link when in project's directory). Linking is for devs-only. If linked, remember to remove from the npm's global cache when done testing by hand (I forgot and lost some time to weird behavior 😅)

I saw that someone has published a fork of this repo, but it has many (unwanted?) changes. Hope this problem will be resolved soon. create-nue is a nice and clean name 👍

Also, due to the already published create-nue package, there may be a name conflict and linking this PR may not work. In that case change name and the key of "./scripts/bin.js" in package.json to something else, for example create-nue-app

Changes

To focus only on the CLI part, I did not change the folder structure. I placed all scripts in /scripts, except for index.js.

  • Changed all scripts (minify.js, render.js, compile.js, server.js) to exported functions. This is a breaking change. They are called via create() or the nue CLI.
  • Added create.js with a function create() that calls all of the scripts listed above
  • Added cli.js which is an entry point for the nue CLI. It accepts inline tags: a script name and options. A script name is one of the above, including create. Options are not yet parsed (they are a topic to discuss).
  • Added bin.js which is an entry point binary for the package (called via npx create-nue, etc). It parses user prompts and and then scaffolds the template
  • Add index.js to export the create() function
  • All npm scripts now either use the nue CLI or execute cli.js directly. The latter is an equivalent to what was done previously, so there shouldn't be new errors. The main use of cli.js is to parse options (which is not yet implemented)

The reason for changing scripts to functions is to implement logic for parsing options in one place (for now it's cli.js). We could just have a function for parsing options, but having one file instead is a good pattern for CLI, and maybe a nue-cli package in the future.

By the way, nice project! 👍

BREAKING CHANGE: Cannot run scripts by executing files, like so:
`$ ./scripts/minify.js`
or with node
`$ node ./scripts/minify.js`
An entry point for CLI.
Allows executing functions as scripts, like so:
`$ ./cli.js minify`
Instead of:
`$ ./minify.js`
`create` function allows to scaffold the project imperatively by
invoking it
Allows to import from the npm package
Scripts now use `nue` CLI.
CLI can also be run in terminal, like so:
`$ nue minify`
Instead of:
`$ npm run minify`, etc...
`bin.js` is an entry point binary of the package.
It:
- welcomes,
- invokes and reads template prompts,
- scaffolds the template
TEMPORARY: New features scaffold a placeholder instead of a template.
Does NOT break the previous method:
`$ bun --bun start`
@tipiirai
Copy link
Contributor

WOW. This is indeed a well-crafted pull request! I originally wanted this repository to be created with just the command npm create nue but I had troubles pushing it live and I had simultaneous pressures to push the whole thing in production so I decided to do this later. But looks like you did it! Big thank you! Merging this now to master, but I see we need a dev- branch, which I will create too.

@tipiirai
Copy link
Contributor

@kon-pas I just pushed out a dev branch where you can continue with this.

pnpm link and pnpm create nue worked perfectly, but then the pnpm start command in the new project directory raised this error "sh: ./scripts/minify.js: Permission denied". Do we need to prefix all commands with node?

I will publish a new npm version after we solve this one.

Thank you!

@tipiirai
Copy link
Contributor

@kon-pas I'm having a hard time getting create-nue to work after merging this. Can you help with finalizing this PR?

@kon-pas
Copy link
Contributor Author

kon-pas commented Sep 27, 2023

@tipiirai Sorry for not posting.
I was working to finalize the template scaffolding. With the new PR #18 this should work, at least for Linux. create-nue has to be published, but until then you can use create-nue-app which is a copy of this PR #18. I will delete/deprecate create-nue-app once create-nue package works.

@kon-pas
Copy link
Contributor Author

kon-pas commented Sep 27, 2023

@tipiirai If you are having any issues, please post errors logs and I will try to resolve it as soon, as possible.

"sh: ./scripts/minify.js: Permission denied". Do we need to prefix all commands with node?

I don't think that's the case. We prefixed with node to run the script on Windows (which I believe the CLI from the new PR #18 resolved). If you are still having the Permission denied issue, try changing the permission of all scripts. chmod a+x <script> should work. I did this in PR #18 but who knows.

@tipiirai
Copy link
Contributor

Please check the latest master / v0.1.4. I think it contains all the stuff on this PR. Unfortunately, I get "403 Forbidden - PUT https://registry.npmjs.org/create-nue - create-nue cannot be republished until 24 hours have passed." error when pushing to NPM, so I need to wait for that still. Check #9

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

Successfully merging this pull request may close these issues.

2 participants