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

create-neon #690

Merged
merged 25 commits into from
Mar 10, 2021
Merged

create-neon #690

merged 25 commits into from
Mar 10, 2021

Conversation

dherman
Copy link
Collaborator

@dherman dherman commented Mar 1, 2021

Initial implementation of the create-neon CLI, as described in the Streamlined UX RFC.

image

@ArturAralin
Copy link

Hi @dherman! Look on this PR neon-bindings/create-neon-lib#1. It may be useful for it

create-neon/templates/Cargo.toml.hbs Outdated Show resolved Hide resolved
create-neon/package.json Show resolved Hide resolved
"bin": "./dist/bin/create-neon.js",
"files": [
"dist/**/*",
"templates/**/*"

This comment was marked as duplicate.

create-neon/src/bin/create-neon.ts Outdated Show resolved Hide resolved
create-neon/data/versions.json Outdated Show resolved Hide resolved
create-neon/src/bin/create-neon.ts Outdated Show resolved Hide resolved
create-neon/src/bin/create-neon.ts Outdated Show resolved Hide resolved
create-neon/src/bin/create-neon.ts Outdated Show resolved Hide resolved
create-neon/src/bin/create-neon.ts Outdated Show resolved Hide resolved
create-neon/src/template.ts Outdated Show resolved Hide resolved
dherman added 2 commits March 4, 2021 23:07
- Abstracted handlebars template logic into a Template class
- Abstracted the template context type into a Metadata module with some type definitions
- Better error messages
- Infer the N-API version from the current process
- Drop the Neon patch version number from the generated manifest
- Use JSON.stringify for quoting text
// we can use, so we have to guess based on the default value. This also
// unfortunately leaks to the user when `npm init` shows the values for
// the package.json it's going to use in the final user confirmation.
if (!/\s*echo \".*\" && exit 1\s*/.test(json.scripts.test)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we write a package.json before executing npm init that contains { "scripts": { "test": "cargo test" } }, it won't prompt for a test command. I think this might be a cleaner workaround than matching on the default text because the user won't even be prompted.

}

constructor(json: any, cargoCpArtifact: string) {
this.json = json;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we have two copies of the data? Overall, I find the class base approach a little complicated. Alternatively,

interface PackageJson {
    name: string,
    author: string,
    [k: string]: any,
}

There might be a package.json types file we could import.

Copy link
Member

@kjvalencik kjvalencik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking awesome! I'm really excited! ✨

create-neon/test/create-neon.ts Show resolved Hide resolved
create-neon/test/create-neon.ts Outdated Show resolved Hide resolved
create-neon/src/bin/create-neon.ts Outdated Show resolved Hide resolved
create-neon/test/create-neon.ts Outdated Show resolved Hide resolved
create-neon/test/create-neon.ts Show resolved Hide resolved
- Delete test output directory after every test
- Add test output directories to gitignore
- Add npm-debug.log to gitignore template
Copy link
Member

@kjvalencik kjvalencik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dherman This looks really good! I like that we start with a fully functional package.json. This way even if the user cancels the prompts, they still end up with a fully functional project.

@dherman
Copy link
Collaborator Author

dherman commented Mar 10, 2021

I don't know enough about node on windows to know if this is always npm.cmd. If we spawn a shell (by passing shell: true to spawn), does it work on Windows without this?

Good thought, I'm running that approach through CI to see if it works on Windows. Seems a bit more robust if so.

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.

3 participants