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

"react-scripts" Required For TypeScript, Even When Using "--scripts-version" #5811

Closed
devloco opened this issue Nov 15, 2018 · 4 comments
Closed

Comments

@devloco
Copy link

devloco commented Nov 15, 2018

Is this a bug report?

Yes

Did you try recovering your dependencies?

Yes

Which terms did you search for in User Guide?

I didn't see anything that looked relevant.

Environment

Uh.. I get an exception running that command. But create-react-app itself works fine, creating an app. This exception doesn't seem to be the culprit. I'm happy to try other commands. I'm assuming this exception is a known problem.

  1. I'm on Windows 10
  2. Normal Windows cmd prompt
  3. npx create-react-app --info
  4. Here's the exception, but like I said, this error isn't the culprit to my actual issue.

`
Environment Info:
(node:3520) UnhandledPromiseRejectionWarning: Error: The system cannot find the path specified.

at Function.e.exports.sync (C:\Users\baez\AppData\Roaming\npm-cache\_npx\3520\node_modules\create-react-app\node_modules\envinfo\dist\envinfo.js:1:7778)
at Object.copySync (C:\Users\baez\AppData\Roaming\npm-cache\_npx\3520\node_modules\create-react-app\node_modules\envinfo\dist\envinfo.js:1:104976)
at Object.t.writeSync.e [as writeSync] (C:\Users\baez\AppData\Roaming\npm-cache\_npx\3520\node_modules\create-react-app\node_modules\envinfo\dist\envinfo.js:1:123499)
at C:\Users\baez\AppData\Roaming\npm-cache\_npx\3520\node_modules\create-react-app\node_modules\envinfo\dist\envinfo.js:1:124274
at Promise.all.then.e (C:\Users\baez\AppData\Roaming\npm-cache\_npx\3520\node_modules\create-react-app\node_modules\envinfo\dist\envinfo.js:1:124289)
at <anonymous>
at process._tickCallback (internal/process/next_tick.js:188:7)

(node:3520) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:3520) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
`

Steps to Reproduce

Use create-react-app to create a new app, use the --scripts-version arg and the --typescript arg.
My own scripts fork is WordPress specific, so you might not be setup to use it. I believe the problem will appear with any --scripts-version arg. But here are my specific steps:

create-react-wptheme
my react-scripts fork (branch)

  1. npx create-react-wptheme foo --typescript
  2. cd foo/react-src
  3. npm run start (sets up the initial WP theme files)
  4. Launch WordPress and select this theme. Be sure to click "view site"
  5. Back at your command prompt: npm run start
  6. See TypeScript compile error about "Can not find module ./logo.svg"
  7. End NPM
  8. npm install react-scripts
  9. npm run start
  10. TypeScript compiles with no errors.

Expected Behavior

Step #6 above should show a successful TypeScript compile.

Actual Behavior

There's a file called "react-app-env.d.ts" inside the src folder when using the --typescript command arg. In there is a single line:
/// <reference types="react-scripts" />

So looks like maybe two possible solutions to the problem:

  1. Always install original "react-scripts" when --typescript is specified, even if --scripts-version is specified.
  2. Change the line in "react-app-env.d.ts" to point at the --scripts-version scripts. I personally don't like this option as it makes maintaining a simple fork that much harder.

Reproducible Demo

(Paste the link to an example project and exact instructions to reproduce the issue.)

@Timer
Copy link
Contributor

Timer commented Nov 15, 2018

This is completely controlled by your fork and should be updated accordingly.

@Timer Timer closed this as completed Nov 15, 2018
@Timer
Copy link
Contributor

Timer commented Nov 15, 2018

I'd take a PR making this smarter via reading from package.json.

@devloco
Copy link
Author

devloco commented Nov 15, 2018

@Timer thanks for pointing me in the right direction.

As for the PR, I'd love to do it, because it would be super-cool to get a PR accepted into create-react-app.

I spent a little time looking at, and it seems the only way to get the scripts name from package.json would be to parse the dependencies, and then to try and figure out which one it is. Seems risky... error prone.

I think to get it right, it would be better to pass the packageName from createReactApp.js into init.js and from there pass it into verifyTypeScriptSetup.

@lock lock bot locked and limited conversation to collaborators Jan 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants