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

Should it be allowed paths on the --scripts-version argument? #4266

Closed
mAiNiNfEcTiOn opened this issue Apr 6, 2018 · 3 comments
Closed

Should it be allowed paths on the --scripts-version argument? #4266

mAiNiNfEcTiOn opened this issue Apr 6, 2018 · 3 comments

Comments

@mAiNiNfEcTiOn
Copy link

At work I was creating our own react-scripts package, but to test it I needed to define a path rather than a package name.

So for that I ran create-react-app my-example-app --scripts-version file:/my/absolute/path

Obviously, this is a misuse of the attribute and I know this is not a bug, it's just how things are 😉

On the other hand, I noticed that the only reason why this didn't work was because in this line on createReactApp.js it is expected that packageName is a part of the path and not the path itself...

So in the beginning of this function I did a mutation of the argument (I know I know, bad boy...):

packageName = packageName.startsWith('file:') ? path.basename(packageName.substr(5)) : packageName;

I'd like to highlight that this serves only my purpose...

So my question is... Imagining that I'd refactor it to a proper fix, that would imply I detect (on the failing places) that a the packageName.startsWith('file:'), then I use as the start of the path.resolve() the packageName directly. Why? To cover cases where the packageName is something like /my/absolute/path/@org/react-scripts, where a basename would kill it 😛

Would you be interested in it?

@mAiNiNfEcTiOn mAiNiNfEcTiOn changed the title Should it be allowed relative paths on the --scripts-version argument? Should it be allowed paths on the --scripts-version argument? Apr 6, 2018
@mAiNiNfEcTiOn
Copy link
Author

mAiNiNfEcTiOn commented Apr 6, 2018

Hmmm... maybe a more important question... is the --scripts-version the right param flag to use? I mean, the version part of it makes me doubt if it should be used for a path or even a different packageName.

@iansu
Copy link
Contributor

iansu commented Apr 10, 2018

You can already use file:... with --scripts-version. It's implemented here: https://github.com/facebook/create-react-app/blob/next/packages/create-react-app/createReactApp.js#L397

We also added some additional documentation about using file:... here: #4015

@iansu iansu closed this as completed Apr 10, 2018
@mAiNiNfEcTiOn
Copy link
Author

@iansu that is true with relative paths, because they can be resolved. What about absolute paths?

E.g.: file:/my/absolute/path/to/react-scripts?

@lock lock bot locked and limited conversation to collaborators Jan 19, 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