Scoped packages on forked versions

[danielfigueiredo]

Hello, everyone!

I know we had a PR merged to accept NPM scoped packages in --scripts-version, this is another issue. I recently have been maintaining a forked version of create-react-app where I mostly customized webpack configs to add things like cssnext, stylelint, etc. The problem happened when I updated ./packages/react-scripts/package.json to be a scoped package having the name to be something like @myOrg/my-package-name.

We all know that scoped packages will create a folder between node_modules and react-scripts, which is perfectly fine! I had of course to do some tweaks in few relative paths and everything worked.

The real problem is not with my published NPM package version, but instead with my local scripts. I'm trying to keep react-scripts e2e tests and things working, so that I can automate my fork and make sure the experience with my forked package is always good (things always working).

Looking at ./tasks/cra.sh around line 66 we have something like:

# Finally, pack react-scripts
scripts_path=$root_path/packages/react-scripts/`npm pack`

So our buddy NPM will look at my scoped package.json:

{
  "name": "@danorg/react-scripts",
  "version": "0.0.1",
...
}

and will generate a tar file named danorg-react-scripts-0.0.1.tgz. Well, things go fine until right after ./packages/create-react-app/index.js performs the install and tries to locate the installed react-scripts folder.

The getPackageName function does:

// Extract package name from tarball url or path.
function getPackageName(installPackage) {
  if (installPackage.indexOf('.tgz') > -1) {
    // The package name could be with or without semver version, e.g. react-scripts-0.2.0-alpha.1.tgz
    // However, this function returns package name only without semver version.
    return installPackage.match(/^.+\/(.+?)(?:-\d+.+)?\.tgz$/)[1];
  } else if (installPackage.indexOf('@') > 0) {
    // Do not match @scope/ when stripping off @version or @tag
    return installPackage.charAt(0) + installPackage.substr(1).split('@')[0];
  }
  return installPackage;
}

It infers the package name from the tar file, so it thinks the folder is danorg-react-scripts but it is actually danorg/react-scripts.

I'm having problems trying to figure out the best way to fix this, my thoughts were that cra.sh and ..../create-react-app/index.js should be a little bit smarter to figure out the actual package name.

A stupid way I found to do it is sending the actual package.json name as an argument to the script.
Let's say that our cra.sh as now the following line:

...
# Finally, pack react-scripts
scripts_path=$root_path/packages/react-scripts/`npm pack`
packageName=`node -e 'console.log(require("./package.json").name)'`

...
node packages/create-react-app/index.js --scripts-version=$scripts_path "$@" --package-name=$packageName

But I don't really know if that is the right way to fix this, maybe I shouldn't be using scoped packages.

[gaearon]

@EnoahNetzach Do you have any thoughts on this?

[EnoahNetzach]

I don't particularly like to add another cli argument.

We could enhance create-react-app/index.js to look for directories too when the packageName contains (but doesn't begin with) "react-script"..

By the way, @danielfigueiredo, is this what you mean?

[danielfigueiredo]

I get the error after yarn install:

daniel@dan-pc ~/Projects> ./create-react-app/tasks/cra.sh ~/Projects/my-test
+ cd ..
+ root_path=/Users/daniel/Projects/create-react-app
+ /Users/daniel/Projects/create-react-app/node_modules/.bin/lerna bootstrap
Lerna v2.0.0-beta.30
Independent Versioning Mode
Bootstrapping 5 packages
Installing external dependencies
Symlinking packages and binaries
Prepublishing packages
Successfully bootstrapped 5 packages.
+ cd packages/react-scripts
+ cp package.json package.json.orig
+ node /Users/daniel/Projects/create-react-app/tasks/replace-own-deps.js
Replaced local dependencies.
++ npm pack
+ scripts_path=/Users/daniel/Projects/create-react-app/packages/react-scripts/myorg-react-scripts-0.0.5.tgz
+ rm package.json
+ mv package.json.orig package.json
+ yarn cache clean
yarn cache v0.18.1
success Cleared cache.
✨  Done in 4.55s.
+ cd /Users/daniel/Projects/create-react-app
+ node packages/create-react-app/index.js --scripts-version=/Users/daniel/Projects/create-react-app/packages/react-scripts/myorg-react-scripts-0.0.5.tgz /Users/daniel/Projects/my-test
Creating a new React app in /Users/daniel/Projects/my-test.

Installing packages. This might take a couple minutes.
Installing react-scripts...
# ...
# later
# ...
Error: Cannot find module '/Users/daniel/Projects/my-test/node_modules/myorg-react-scripts/package.json'
    at Function.Module._resolveFilename (module.js:470:15)
    at Function.Module._load (module.js:418:25)
    at Module.require (module.js:498:17)
    at require (internal/module.js:20:19)
    at checkNodeVersion (/Users/daniel/Projects/create-react-app/packages/create-react-app/index.js:229:21)

So it can't find the module because the folder structure is:

node_modules/
  |-- myorg/
    |-- react-scripts/

I'm really sorry if I'm doing something wrong and I'm consuming your time

[danielfigueiredo]

We could enhance create-react-app/index.js to look for directories too when the packageName contains (but doesn't begin with) "react-script"..

I haven't thought about that, it is a good idea. The only problem I see is that if you have a scoped package that is a fork of react-script but doesn't have react-script in the name, like @myorg/react-super-awesome-scripts-:P it wouldn't work.

That's why I thought that the .sh could add a parameter to the script automatically, so the API for the user would still be same, but I definitely lack proper knowledge in how to structure things in the CRA.

[gaearon]

I think relying on package name could be confusing and unexpected.

[EnoahNetzach]

@danielfigueiredo have you checked #1537 out?

With this approach any fork must have -react-scripts in its name, but I think it's a viable solution.

[danielfigueiredo]

@EnoahNetzach I just rebased from facebook and it works!
Thanks a lot