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

Unable to build project #1

Closed
kenk2 opened this issue Jun 20, 2019 · 3 comments
Closed

Unable to build project #1

kenk2 opened this issue Jun 20, 2019 · 3 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@kenk2
Copy link
Contributor

kenk2 commented Jun 20, 2019

This is just a direct escalation as I'm trying to find out how to make nextjs work with Preact X. As I noted here, I was unable to build your repository. I cloned from the master branch and started with "yarn run build" and came up with the error of not being able to find React:

$ next build
The module 'react' was not found. Next.js requires that you include it in 'dependencies' of your 'package.json'. To add it, run 'npm install --save react'
The module 'react-dom' was not found. Next.js requires that you include it in 'dependencies' of your 'package.json'. To add it, run 'npm install --save react-dom'
internal/modules/cjs/loader.js:670
    throw err;
    ^

Error: Cannot find module 'react'

@SaltyAom
Copy link
Owner

SaltyAom commented Jun 21, 2019

As commit of ef16370, I update a trick that add react as dev-dependencies to trick Next.js, but we actually use Preact.

@SaltyAom SaltyAom added the bug Something isn't working label Jun 21, 2019
@SaltyAom
Copy link
Owner

I’ll leave this issue opened, as I am still not sure if we could build without React.js.

@SaltyAom SaltyAom added the good first issue Good for newcomers label Jun 21, 2019
@kenk2
Copy link
Contributor Author

kenk2 commented Jun 22, 2019

Hi @aomkirby123

So I went out and tried the repo, and it worked great! Except a few things I noted:

  • The scripts broke on my end because SET is not a valid keyword. I assume you used this on windows since that was where the SET keyword is but I think a lot of developers also develop on macOS as well as Linux. There's the cross-env for setting environment variables in scripts if you want it to work on both MS-DOS and linux/unix shells. The resulting error I got when trying to run the dev and start scripts:
[kenny@kenny-pc nextjs-preactX]$ yarn run start
yarn run v1.16.0
$ SET NODE_ENV=production && node server.js
/bin/sh: SET: command not found
error Command failed with exit code 127.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
[kenny@kenny-pc nextjs-preactX]$ 

You could probably replace the start and dev scripts to look like this:

  "scripts": {
    "dev": "cross-env NODE_ENV=development node server.js",
    "build": "next build",
    "start": "cross-env NODE_ENV=production node server.js"
  },
  • This repo will actually break with the beta.3 if you try using hooks. Since you aren't, it should be okay as it stands. Beta.2 works for hooks for now but if you upgrade to beta.3, beware if you try to use the new preact hooks API using this as a template for future projects.

Overall you've done a great job here. These are more or less optional to do as you could probably just note to us an MS-DOS shell for the scripts and avoid using beta.3 for stability purposes but otherwise this is pretty much good.

UPDATE: I've submitted a pull request that updates the scripts to be a little more system compatible. The second point can be ignored for now while I escalate this issue with the preact team.

UPDATE 2:
Looks like marvin already has the solution to this: #3

@kenk2 kenk2 closed this as completed Jun 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants