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

Gulp bundle command exit with code '1' for eslint warnings #9165

Closed
9 tasks
HuihuiWu-Microsoft opened this issue Aug 28, 2023 · 4 comments
Closed
9 tasks

Gulp bundle command exit with code '1' for eslint warnings #9165

HuihuiWu-Microsoft opened this issue Aug 28, 2023 · 4 comments
Labels
area:tooling Category: Development tooling Needs: Author Feedback Awaiting response from the original poster of the issue. Marked as stale if no activity for 7 days.

Comments

@HuihuiWu-Microsoft
Copy link

Target SharePoint environment

SharePoint Online

What SharePoint development model, framework, SDK or API is this about?

💥 SharePoint Framework

Developer environment

Windows

What browser(s) / client(s) have you tested

  • 💥 Internet Explorer
  • 💥 Microsoft Edge
  • 💥 Google Chrome
  • 💥 FireFox
  • 💥 Safari
  • mobile (iOS/iPadOS)
  • mobile (Android)
  • not applicable
  • other (enter in the "Additional environment details" area below)

Additional environment details

  • browser version
  • SPFx version 1.17.4
  • Node.js version 16.18.0
  • etc

Describe the bug / error

gulp bundle failed with exit code 1 even if there's only eslint warnings.

Steps to reproduce

  1. scaffold a spfx project with 'yo @microsoft/sharepoint'
  2. customize the code to include eslint error
  3. run gulp bundle --ship --no-color after npm install
  4. process exited with code 1 even if the eslint is only printed as warning
    image

Expected behavior

The behavior should be aligned: if eslint errors are printed as warning, exit code should be 0; if eslint errors are printed as error, exit code should be 1.

@HuihuiWu-Microsoft HuihuiWu-Microsoft added the type:bug-suspected Suspected bug (not working as designed/expected). See “type:bug-confirmed” for confirmed bugs. label Aug 28, 2023
@ghost
Copy link

ghost commented Aug 28, 2023

Thank you for reporting this issue. We will be triaging your incoming issue as soon as possible.

@ghost ghost added the Needs: Triage 🔍 Awaiting categorization and initial review. label Aug 28, 2023
@HuihuiWu-Microsoft
Copy link
Author

This is related to an IcM incident that user is using Teams Toolkit to deploy SPFx but failed. The root cause is that there's eslint error in user's customized code and when user deploy with TTK, TTK will run gulp bundle but failed since the exit code is 1.

User is complaining that it shouldn't block since it's displayed as warning. It's quite misleading for customers.

@nick-pape
Copy link
Collaborator

nick-pape commented Aug 29, 2023

@HuihuiWu-Microsoft

Interesting. So yes, by default, linter warnings are treated as build-blocking errors when running in --ship. This is generally a good practice, otherwise builds would not be blocked and lint errors would proliferate in the codebase. In other words, there's no point in running the linter if the user is going to just ignore the errors. Without the --ship flag, we do not treat linter as an error because it is highly annoying that one cannot test their code (in their inner dev loop) because of lint errors.

There's a few things the user can do here:

Best solutions:

  • Fix the code such that it does not have a lint warning. Using any is error prone and should be avoided.
  • Add an eslint override comment to the code so that the linter ignores the warning (// eslint-disable-next-line @typescript-eslint/no-explicit-any)

Less ideal solutions:

  • Make lint warnings always be treated as build blocking errors (even without --ship).
    • Note this could be annoying during a developer's inner loop.
    • Add a file called lint.json in the config/ folder. Add the following:
      { displayAsError: true; }
      
    • Alternately, update gulpfile.js and add:
      build.lintCmd.setConfig({ displayAsError: true });
      
    • See here.

Solutions that work but are not recommended:

  • Add an override to the build to ignore the error:
    • In gulpfile.js add use the build.addSuppression(); which accepts a RegEx or a string).
    • See here.
  • Disable the rule in the project's .eslintrc.js config
  • Disable the linter entirely (in gulpfile.js add build.lintCmd.enabled = false;)

However, I will say this is default behavior that we are not planning to change.

@nick-pape nick-pape added area:tooling Category: Development tooling Needs: Author Feedback Awaiting response from the original poster of the issue. Marked as stale if no activity for 7 days. and removed type:bug-suspected Suspected bug (not working as designed/expected). See “type:bug-confirmed” for confirmed bugs. Needs: Triage 🔍 Awaiting categorization and initial review. labels Aug 29, 2023
@ghost
Copy link

ghost commented Sep 14, 2023

Issues that have been closed & had no follow-up activity for at least 7 days are automatically locked. Please refer to our wiki for more details, including how to remediate this action if you feel this was done prematurely or in error: Issue List: Our approach to locked issues

@ghost ghost locked as resolved and limited conversation to collaborators Sep 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area:tooling Category: Development tooling Needs: Author Feedback Awaiting response from the original poster of the issue. Marked as stale if no activity for 7 days.
Projects
None yet
Development

No branches or pull requests

2 participants