Repository: netlify/build
Pull request #3925 (2021-12-03 13:01:02)

feat: pass additional feature flags to zip-it-and-ship-it
#### Summary

This PR is divided into two parts. Firstly, we pass a `parseISC` feature flag to zip-it-and-ship-it when the `zisi_parse_isc` LaunchDarkly flag is defined. This is the usual process of adding a new feature flag, because there is some translation happening between LaunchDarkly flags and zip-it-and-ship-it flags.

This process is getting a bit cumbersome though, as @ehmicky rightly pointed out a while ago. For this reason, this PR also starts passing all feature flags to zip-it-and-ship-it.

This means we'll need to start adopting a new convention for feature flag naming in zip-it-and-ship-it (i.e. using snake case 🐍  instead of camel case 🐫 ), and probably using a zip-it-and-ship-it prefix (i.e. `zisi_some_flag` as opposed to `buildbot_some_flag`).

We'll keep the existing flags as they are until we decommission them, but going forward we should be able to add new zip-it-and-ship-it flags without adding them explicitly to Netlify Build.

**A picture of a cute animal (not mandatory, but encouraged)**

![sloth-plush](https://user-images.githubusercontent.com/4162329/144606670-c35f1ae8-e75b-42c2-b3b6-e21f26e0f311.jpg)





Patch:
diff --git a/.eslintrc.js b/.eslintrc.js
index a557fd532..5413343b6 100644
--- a/.eslintrc.js
+++ b/.eslintrc.js
@@ -86,5 +86,11 @@ module.exports = {
         'node/no-extraneous-import': 0,
       },
     },
+    {
+      files: ['packages/*/tests/**/*.js'],
+      rules: {
+        'no-magic-numbers': 'off',
+      },
+    },
   ],
 }
diff --git a/packages/build/src/core/feature_flags.js b/packages/build/src/core/feature_flags.js
index 5f1010fcc..7d4283b30 100644
--- a/packages/build/src/core/feature_flags.js
+++ b/packages/build/src/core/feature_flags.js
@@ -20,6 +20,7 @@ const DEFAULT_FEATURE_FLAGS = {
   buildbot_zisi_trace_nft: false,
   buildbot_zisi_esbuild_parser: false,
   buildbot_scheduled_functions: false,
+  zisi_parse_isc: false,
 }
 
 module.exports = { normalizeCliFeatureFlags, DEFAULT_FEATURE_FLAGS }
diff --git a/packages/build/src/plugins_core/functions/feature_flags.js b/packages/build/src/plugins_core/functions/feature_flags.js
index 63f83df67..66e5bbb44 100644
--- a/packages/build/src/plugins_core/functions/feature_flags.js
+++ b/packages/build/src/plugins_core/functions/feature_flags.js
@@ -1,9 +1,11 @@
 'use strict'
 
 const getZisiFeatureFlags = (featureFlags) => ({
+  ...featureFlags,
   buildGoSource: featureFlags.buildbot_build_go_functions,
   defaultEsModulesToEsbuild: featureFlags.buildbot_es_modules_esbuild,
   nftTranspile: featureFlags.buildbot_nft_transpile_esm || featureFlags.buildbot_zisi_trace_nft,
+  parseISC: featureFlags.zisi_parse_isc,
   parseWithEsbuild: featureFlags.buildbot_zisi_esbuild_parser,
   traceWithNft: featureFlags.buildbot_zisi_trace_nft,
 })


Test patch:
diff --git a/packages/build/tests/core/tests.js b/packages/build/tests/core/tests.js
index ee06a6e9d..642f72abb 100644
--- a/packages/build/tests/core/tests.js
+++ b/packages/build/tests/core/tests.js
@@ -228,7 +228,6 @@ const getNodeBinary = async function (nodeVersion, retries = 1) {
 const MAX_RETRIES = 10
 
 // Memoize `get-node`
-// eslint-disable-next-line no-magic-numbers
 const mGetNode = moize(getNodeBinary, { isPromise: true, maxSize: 1e3 })
 
 test('--node-path is used by build.command', async (t) => {
@@ -380,27 +379,37 @@ test.serial('Passes the right feature flags to zip-it-and-ship-it', async (t) =>
     flags: { featureFlags: { buildbot_nft_transpile_esm: true } },
     snapshot: false,
   })
+  await runFixture(t, 'schedule', {
+    flags: { featureFlags: { zisi_parse_isc: true } },
+    snapshot: false,
+  })
+  await runFixture(t, 'schedule', {
+    flags: { featureFlags: { this_is_a_mock_flag: true, and_another_one: true } },
+    snapshot: false,
+  })
 
   stub.restore()
 
-  // eslint-disable-next-line no-magic-numbers
-  t.is(mockZipFunctions.callCount, 6)
+  t.is(mockZipFunctions.callCount, 8)
 
   t.false(mockZipFunctions.getCall(0).args[2].featureFlags.traceWithNft)
   t.false(mockZipFunctions.getCall(0).args[2].featureFlags.buildGoSource)
   t.false(mockZipFunctions.getCall(0).args[2].featureFlags.parseWithEsbuild)
+  t.is(mockZipFunctions.getCall(0).args[2].config.test.schedule, undefined)
   t.false(mockZipFunctions.getCall(0).args[2].featureFlags.nftTranspile)
+  t.false(mockZipFunctions.getCall(0).args[2].featureFlags.parseISC)
+  t.is(mockZipFunctions.getCall(0).args[2].featureFlags.this_is_a_mock_flag, undefined)
+  t.is(mockZipFunctions.getCall(0).args[2].featureFlags.and_another_one, undefined)
 
   t.true(mockZipFunctions.getCall(1).args[2].featureFlags.traceWithNft)
   t.true(mockZipFunctions.getCall(1).args[2].featureFlags.nftTranspile)
   t.true(mockZipFunctions.getCall(2).args[2].featureFlags.buildGoSource)
   t.true(mockZipFunctions.getCall(3).args[2].featureFlags.parseWithEsbuild)
-  // eslint-disable-next-line no-magic-numbers
-  t.true(mockZipFunctions.getCall(5).args[2].featureFlags.nftTranspile)
-
-  t.is(mockZipFunctions.getCall(0).args[2].config.test.schedule, undefined)
-  // eslint-disable-next-line no-magic-numbers
   t.is(mockZipFunctions.getCall(4).args[2].config.test.schedule, '@daily')
+  t.true(mockZipFunctions.getCall(5).args[2].featureFlags.nftTranspile)
+  t.true(mockZipFunctions.getCall(6).args[2].featureFlags.parseISC)
+  t.true(mockZipFunctions.getCall(7).args[2].featureFlags.this_is_a_mock_flag)
+  t.true(mockZipFunctions.getCall(7).args[2].featureFlags.and_another_one)
 })
 
 test('Print warning on lingering processes', async (t) => {
diff --git a/packages/build/tests/telemetry/tests.js b/packages/build/tests/telemetry/tests.js
index 64158aff8..848a8a0df 100644
--- a/packages/build/tests/telemetry/tests.js
+++ b/packages/build/tests/telemetry/tests.js
@@ -224,7 +224,6 @@ test('Telemetry calls timeout by default', async (t) => {
     // We want to rely on the default timeout value
     disableTelemetryTimeout: false,
     // Introduce an arbitrary large timeout on the server side so that we can validate the client timeout works
-    // eslint-disable-next-line no-magic-numbers
     waitTelemetryServer: 5 * 60 * 1000,
     // The error monitor snapshot should contain the timeout error
     snapshot: true,