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

verifyNoTypeScript false positives #5947

Closed
holloway opened this issue Dec 3, 2018 · 5 comments
Closed

verifyNoTypeScript false positives #5947

holloway opened this issue Dec 3, 2018 · 5 comments

Comments

@holloway
Copy link
Contributor

holloway commented Dec 3, 2018

Is this a bug report?

Yes (I think)

However I am just looking for clarification about how to structure projects with CRA. Should I use yarn link instead of symlinks?

Which terms did you search for in User Guide?

"sym" "symlink" "symbolic"

Steps to Reproduce

  1. symlink ./src/anything to a directory outside the CRA project directory called anything.
  2. Initialise anything with its own package.json that depends on big-integer. big-integer is distributed on NPM with TypeScript files which will be present at anything/node_modules/big-integer/BigInteger.d.ts.
  3. Run yarn build and CRA tells me that my project has TypeScript in it and refuses to build.

Expected Behavior

That CRA build wouldn't look in node_modules from symlinked directories.

Unfortunately globby doesn't seem to have an option to ignore symlinks.

Actual Behavior

We detected TypeScript in your project (src/anything/node_modules/big-integer/BigInteger.d.ts) and created a tsconfig.json file for you.

and the build fails because it's not TypeScript

@mrmckeb
Copy link
Contributor

mrmckeb commented Dec 8, 2018

Hmmm, interesting problem, @holloway.

One option is to improve the verifyNoTypeScript script to filter out matches that include node_modules - can you see any issues with that? A better option could be to check for symlinks, but that could get complicated and expensive.

@holloway
Copy link
Contributor Author

holloway commented Dec 10, 2018

@mrmckeb Yes, I think filtering node_modules would be an appropriate fix, and would likely have no side-effects. I think you're right that checking for symlinks could be expensive/slow, especially in a build process that everyone runs...besides, anyone putting a node_modules in their src with or without symlinks probably wouldn't expect verifyNoTypeScript to check in there so filtering node_modules does seem appropriate.

Perhaps another approach might be to have a config option but that might go against the ethos of CRA. Anyway, that could look like,

We detected TypeScript in your project (src/anything/node_modules/big-integer/BigInteger.d.ts) and created a tsconfig.json file for you. If this is incorrect add "src-language": "javascript" to your package.json.

@mrmckeb
Copy link
Contributor

mrmckeb commented Dec 11, 2018

I actually think that long-term, we might need to have some kind of config, but understand why this isn't in place now. We'll keep that in mind while planning for the future.

Did you want to create a PR for this @holloway and assign to me? Otherwise, I could try to look at creating a PR this weekend.

@stale
Copy link

stale bot commented Jan 12, 2019

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Jan 12, 2019
@stale
Copy link

stale bot commented Jan 18, 2019

This issue has been automatically closed because it has not had any recent activity. If you have a question or comment, please open a new issue.

@stale stale bot closed this as completed Jan 18, 2019
@lock lock bot locked and limited conversation to collaborators Jan 23, 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