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

Add flags and dead code elimination #53

Closed
gaearon opened this issue Jul 21, 2016 · 4 comments
Closed

Add flags and dead code elimination #53

gaearon opened this issue Jul 21, 2016 · 4 comments

Comments

@gaearon
Copy link
Contributor

gaearon commented Jul 21, 2016

Hacks like #52 are frustrating.
We need a system that lets us write stuff like

if (__REPO__) {
  var path = ...
} else if (__DEPENDENCY__) {
  var path = ...
} else if (__EJECTED__) {
  var path = ...
}

and eliminate all other branches on eject.

@Jiansen
Copy link
Contributor

Jiansen commented Jul 24, 2016

@gaearon , if I understood your concerns correctly, the frustrating part is that code like the following will become more difficult to maintain with the increased number of commands in later versions.

var relativePath = isInNodeModules ? '../../..' : '..';
if (process.argv[2] === '--debug-template') {
  relativePath = '../template';
}

If so, with match-js or a similar approach, I should be able to refactor the code similar to the following, though not exactly in your proposed form

relativePath = match( env )(
  CASE(REPO, ...),
  CASE(DEPENDENCY, ...),
  CASE(EJECTED, ...),
  CASE(otherwise, ...)
);

where env could be process.argv or other value that represents the execution environment.

Currently, I see that --debug-template and --smoke-test are both set at process.argv[2], and there is no check for EJECTED because scripts/eject.js is the only involved script.

To help me understand your problem correctly, could you, from a user's perspective, list some test commands that set flag(s)?

In my forked repo, I will test my ideas. I hope that we can move towards a solution that you wanted.

Cheers

@gaearon
Copy link
Contributor Author

gaearon commented Jul 24, 2016

These flags are all internal and we don’t want to expose this code for the user. They are only useful for testing and local development of create-react-app itself. So if statements are not a problem and we don’t need to refactor them. What we need is a way to cut some code after ejecting.

@Jiansen
Copy link
Contributor

Jiansen commented Jul 24, 2016

@gaearon ,

The above change deletes dead code on ejecting; However, this does not prevent users from using npm start --debug-template before react-scripts is ejected. At lease, we don't expose the flag to general users.

I will think if we could hide the flag and cut off the code on the time of building react-scripts.

If you think the current progress is OK, I will do some tests and create a PR.

@gaearon
Copy link
Contributor Author

gaearon commented Sep 3, 2016

This was in #257.

@gaearon gaearon closed this as completed Sep 3, 2016
@lock lock bot locked and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants