Skip to content

Commit

Permalink
feat: feature flag for eslint migration (appsmithorg#36543)
Browse files Browse the repository at this point in the history
## Description

This PR introduces a feature flag to control the ESLint migration.

Fixes appsmithorg#36542

## Automation

/test js sanity

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/11795470649>
> Commit: ac773e1
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=11795470649&attempt=2"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.JS, @tag.Sanity`
> Spec:
> <hr>Tue, 12 Nov 2024 11:58:31 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [x] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **New Features**
- Introduced a new feature flag, `rollout_eslint_enabled`, for managing
ESLint functionality.
- Added an enumeration for linter versions, including `"JSHINT"` and
`"ESLINT"`.
- Implemented a function to dynamically determine the linter version
based on the feature flag.

- **Bug Fixes**
- Enhanced linting error handling by updating the linter version
dynamically instead of using a hardcoded value.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
ayushpahwa authored Nov 12, 2024
1 parent a1383b2 commit 3e55611
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 1 deletion.
2 changes: 2 additions & 0 deletions src/ce/entities/FeatureFlag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export const FEATURE_FLAG = {
release_actions_redesign_enabled: "release_actions_redesign_enabled",
rollout_remove_feature_walkthrough_enabled:
"rollout_remove_feature_walkthrough_enabled",
rollout_eslint_enabled: "rollout_eslint_enabled",
release_drag_drop_building_blocks_enabled:
"release_drag_drop_building_blocks_enabled",
rollout_side_by_side_enabled: "rollout_side_by_side_enabled",
Expand Down Expand Up @@ -71,6 +72,7 @@ export const DEFAULT_FEATURE_FLAG_VALUE: FeatureFlags = {
ab_appsmith_ai_query: false,
release_actions_redesign_enabled: false,
rollout_remove_feature_walkthrough_enabled: true,
rollout_eslint_enabled: false,
rollout_side_by_side_enabled: false,
release_layout_conversion_enabled: false,
release_anvil_toggle_enabled: false,
Expand Down
5 changes: 5 additions & 0 deletions src/plugins/Linting/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@ import { ECMA_VERSION } from "@shared/ast";
import type { LintOptions } from "jshint";
import isEntityFunction from "./utils/isEntityFunction";

export enum LINTER_TYPE {
"JSHINT" = "JSHint",
"ESLINT" = "ESLint",
}

export const lintOptions = (globalData: Record<string, boolean>) =>
({
indent: 2,
Expand Down
18 changes: 17 additions & 1 deletion src/plugins/Linting/utils/getLintingErrors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
IGNORED_LINT_ERRORS,
lintOptions,
SUPPORTED_WEB_APIS,
LINTER_TYPE,
} from "../constants";
import type { getLintingErrorsProps } from "../types";
import { JSLibraries } from "workers/common/JSLibrary";
Expand All @@ -38,9 +39,23 @@ import { generate } from "astring";
import getInvalidModuleInputsError from "ee/plugins/Linting/utils/getInvalidModuleInputsError";
import { objectKeys } from "@appsmith/utils";
import { profileFn } from "UITelemetry/generateWebWorkerTraces";
import { WorkerEnv } from "workers/Evaluation/handlers/workerEnv";
import { FEATURE_FLAG } from "ee/entities/FeatureFlag";

const EvaluationScriptPositions: Record<string, Position> = {};

function getLinterType() {
let linterType = LINTER_TYPE.JSHINT;

const flagValues = WorkerEnv.getFeatureFlags();

if (flagValues?.[FEATURE_FLAG.rollout_eslint_enabled]) {
linterType = LINTER_TYPE.ESLINT;
}

return linterType;
}

function getEvaluationScriptPosition(scriptType: EvaluationScriptType) {
if (isEmpty(EvaluationScriptPositions)) {
// We are computing position of <<script>> in our templates.
Expand Down Expand Up @@ -194,6 +209,7 @@ export default function getLintingErrors({
scriptType,
webworkerTelemetry,
}: getLintingErrorsProps): LintError[] {
const linterType = getLinterType();
const scriptPos = getEvaluationScriptPosition(scriptType);
const lintingGlobalData = generateLintingGlobalData(data);
const lintingOptions = lintOptions(lintingGlobalData);
Expand All @@ -202,7 +218,7 @@ export default function getLintingErrors({
"Linter",
// adding some metrics to compare the performance changes with eslint
{
linter: "JSHint",
linter: linterType,
linesOfCodeLinted: originalBinding.split("\n").length,
codeSizeInChars: originalBinding.length,
},
Expand Down

0 comments on commit 3e55611

Please sign in to comment.