-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[ISSUE/348] Only display user-defined JS lint errors #745
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR @makinox!
This is a good approach, but there's one comment/request to be addressed before we merge the PR.
"extends": "react-app", | ||
"ignorePatterns": [ | ||
"src/*", | ||
"!src/ext-src" | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea for a fix! I think this is the best short-term solution.
One important thing to note, however. The current approach assumes and hardcodes the ext-src
inside the package.json
file. Wasp's directory structure will most likely go through a lot of changes in the near future and we want to make sure this changes accordingly.
Ideally like to read and interpolate this path from a single source of truth (i.e., here). You can do this using the same templating system that's used for depsChunk
and devDepsChunk
. It involves editing a bit of Haskell code. Let me know if you need any help :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, on my way 🏃♂️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DONE ⚡️, I was going to reuse that path even more, but I watched in your code that you are not doing that with your path variables GeneratedExternalCodeDir
, maybe there is a good reason that I don't know yet, let me know if this solves the issue 🫡😬
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes!
However, we already have an appropriate constant (source of truth) for this. I liked to it in the previous comment. Please use that one (you can move it to WebAppGenerator/Common).
but I watched in your code that you are not doing that with your path variables GeneratedExternalCodeDir
Not sure I got this. Can you elaborate and/or list an example?
Description
Fixes # (348)
Type of change