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

ignore intellij module files when generating an app #4605

Merged
merged 1 commit into from Nov 2, 2018

Conversation

denofevil
Copy link
Contributor

When IntelliJ IDEA generates a project, it creates a module file (*.iml) in the root directory of the project by default. IDEA does that before it launches project generator process which is causing CRA to abort due to these extra files.
We used to workaround that via generating the project in a temporary directory, but that causes another set of problems, such as:
WEB-29489 (aka #200)
WEB-33257
and additionally is just slower due to copying the generated project to the proper location.
It would be better to ignore *.iml files in the generator script.

These changes were checked by generating the project from the terminal and from IntelliJ (with temp dir workaround disabled).

@Timer
Copy link
Contributor

Timer commented Jun 14, 2018

Hmm, I think I'd rather allow regex valid files to be used than adding a special case like in this PR. This will make it more flexible for the future.

What do you think?

@denofevil
Copy link
Contributor Author

Yeah, that would work as well. I was just not sure if there's a reason for not having regexp in valid files except historical one.

Let me add regexp support there

@denofevil
Copy link
Contributor Author

On a second thought, existing paths will require conversion and will look a little bit ugly.

Or we can go with glob patterns. As far as I see glob is already in node_modules, just not explicitly mentioned

@Timer Timer closed this Sep 26, 2018
@Timer Timer reopened this Sep 26, 2018
@gaearon gaearon closed this Nov 1, 2018
@denofevil
Copy link
Contributor Author

Hi @gaearon, I'd still love to get this fixed and merged if you don't have any objections. In order to improve it would be nice to get some feedback on mine questions.

@Timer Timer changed the base branch from next to master November 2, 2018 12:23
@Timer Timer reopened this Nov 2, 2018
@@ -663,6 +662,8 @@ function isSafeToCreateProjectIn(root, name) {
const conflicts = fs
.readdirSync(root)
.filter(file => !validFiles.includes(file))
// IJ creates module files before CRA is launched
.filter(file => file.indexOf('.iml') < 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use regex here for consistency with how we do it elsewhere:

Suggested change
.filter(file => file.indexOf('.iml') < 0)
.filter(file => !/\.iml$/.test(file))

@@ -663,6 +662,8 @@ function isSafeToCreateProjectIn(root, name) {
const conflicts = fs
.readdirSync(root)
.filter(file => !validFiles.includes(file))
// IJ creates module files before CRA is launched
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's spell this out for users that don't know what IntelliJ is.

Suggested change
// IJ creates module files before CRA is launched
// IntelliJ IDEA creates module files before CRA is launched

@Timer Timer added this to the 2.1.2 milestone Nov 2, 2018
@denofevil
Copy link
Contributor Author

Rebased & added requested changes

@Timer Timer merged commit 20be1b6 into facebook:master Nov 2, 2018
@Timer
Copy link
Contributor

Timer commented Nov 2, 2018

Thanks!

@denofevil
Copy link
Contributor Author

Thank you!

xs9627 pushed a commit to xs9627/create-react-app that referenced this pull request Nov 12, 2018
* facebook-master: (578 commits)
  Add PR welcoming badge  (facebook#5759)
  Suggest Encore when not building a SPA with Symfony (facebook#5730)
  Merge webpack configuration (facebook#5722)
  Tweak bot settings
  Update stale.yml
  Fix typo (facebook#5727)
  Version bump postcss-preset-env to latest (facebook#5721)
  Updated the link to firebase hosting (facebook#5710)
  Fixed link to manifest.json file (facebook#5704)
  Fix broken documentation link (facebook#5670)
  Fix tsconfig.json lib suggested value (facebook#5701)
  Add permissive TS lib defaults (facebook#5694)
  Lock issues more aggressively
  Make stale bot configuration more aggressive
  ignore intellij module files when generating an app (facebook#4605)
  update envinfo to 5.11.1 (facebook#5685)
  Add bot config (facebook#4483)
  Publish
  Update cached lock file
  Add changelog for 2.1.1
  ...
@lock lock bot locked and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants