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

--skip-install option in create-next-app #32367

Open
Pika-Pool opened this issue Dec 10, 2021 · 9 comments
Open

--skip-install option in create-next-app #32367

Pika-Pool opened this issue Dec 10, 2021 · 9 comments

Comments

@Pika-Pool
Copy link

@Pika-Pool Pika-Pool commented Dec 10, 2021

Describe the feature you'd like to request

A --skip-install option in create-next-app, like in react-native, to skip installing dependencies would be really great. It allows using other package managers like pnpm

Describe the solution you'd like

do not install dependencies automatically when the --skip-install option is set. But the dependencies should be listed in package.json, so user could manually install the dependencies using other package managers like pnpm

Describe alternatives you've considered

An option to use pnpm(--use-pnpm). pnpm has gained significant popularity and would be a good option to add.

@balazsorban44
Copy link
Member

@balazsorban44 balazsorban44 commented Dec 10, 2021

Adding a --skip-install flag sounds reasonable. I'm not 100% what would go into the --use-pnpm flag, let's start with the first one.

Here are the appropriate files:

(look for await install)
https://github.com/vercel/next.js/blob/canary/packages/create-next-app/create-app.ts

https://github.com/vercel/next.js/blob/canary/packages/create-next-app/index.ts

@anirudh1713
Copy link

@anirudh1713 anirudh1713 commented Dec 11, 2021

Hey there, I'm trying to do this. skipping the installation is not a problem, but what about populating package.json with version of packages? I don't think * or latest is a good idea for version of the packages 🤔

@Pika-Pool
Copy link
Author

@Pika-Pool Pika-Pool commented Dec 11, 2021

Currently, the cli installs "latest" for next and the exact versions for react, react-dom and the dev dependencies

There are a few methods of doing this in my opinion. I'm listing them down:

  1. create a template package.json file and update it whenever the versions update. React-native seems to be doing this
    • disadvantage: have to constantly monitor the latest version of packages
  2. use the npm show or yarn info commands to get the latest version of the respective packages
    • disadvantage: need internet connection
  3. query the npm registry to get the latest version
    • disadvantage: need internet connection
  4. We can just console.log a message with the appropriate command, like: yarn add next react react-dom, just like the cli does with yarn run dev right now, without populating package.json

@balazsorban44, what do you think should be the optimal solution?

PS: I was hoping I could work on this, as I have already started too. Am I too late?

@balazsorban44
Copy link
Member

@balazsorban44 balazsorban44 commented Dec 11, 2021

Feel free to work on adding the --skip-install flag @Pika-Pool.

I'm not sure about what @anirudh1713 is suggesting. The CLI is meant to scaffold a minimal Next.js example project. You can use the --examlpe flag to pull from any of the available examples in this folder.

@Pika-Pool
Copy link
Author

@Pika-Pool Pika-Pool commented Dec 12, 2021

When using the default templates from this folder, the dependency list in package.json is populated using the install command(npm install or yarn install).
When the --skip-install option is enabled, we can no longer use this method. But we still need to list the dependencies, with their versions in the package.json file, so the user can install them using their favorite package manager.

So what would be a good method to get the respective versions? Should we just mark it as latest or *? Or I have listed some other ways I can think of in my previous comment

@anirudh1713
Copy link

@anirudh1713 anirudh1713 commented Dec 12, 2021

@Pika-Pool Yes, that is what I was trying to say in my previous comment, latest or * doesn't seem like a good idea. I think --use-pnpm might be a better way. 🤔 or maybe maintaining a package.json?

@balazsorban44
Copy link
Member

@balazsorban44 balazsorban44 commented Dec 14, 2021

Thank you for the explanation.

The CLI is expected to spin up the latest version of next, so that would be sufficient I believe. You can expect the template code to be always up-to-date for the latest dist tag.

Pika-Pool added a commit to Pika-Pool/next.js that referenced this issue Dec 14, 2021
- adds feature issue vercel#32367
- facilitates using any package manager, not just npm/yarn
- for default templates, writes "latest" dist tag for deps in package.json
@Pika-Pool
Copy link
Author

@Pika-Pool Pika-Pool commented Dec 20, 2021

I've added a PR(#32484) to add this feature. It adds "latest" as the version for each dependency for the default templates.

Tests

For this feature, what all should the tests cover? This is what I think, please add/remove tests as required:

  • check is project is created by checking if files exist
  • node_modules folder does not exists
  • check if all items in Object.values(packageJson.dependencies) and Object.values(packageJson.devDependencies) is latest

Is it necessary to add tests for --example or --typescript flags, along with --skip-install too?

Where to add the tests?

I am not able to find where to add them. There are tests for create-next-app at test/integration/create-next-app/index.test.js but:

testing readme mentions that "integration: These tests run misc checks and modes and is where tests used to be added before we added more specific folders. We should not add any more tests here.".

So where should I add the new tests for --skip-install option?

@balazsorban44
Copy link
Member

@balazsorban44 balazsorban44 commented Dec 22, 2021

Feel free to add it in https://github.com/vercel/next.js/blob/canary/test/integration/create-next-app/index.test.js

We will update the README to avoid future confusion.

kodiakhq bot pushed a commit that referenced this issue Dec 22, 2021
As mentioned in #32679 (comment) it looks like these externals need to be updated to the compiled path not that they are ncc'd. Also updated the test readme a bit to reduce confusion in #32367 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants