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

feat(27255): allow local modification for remote feature flags #29696

Merged
merged 55 commits into from
Feb 21, 2025
Merged
Changes from 1 commit
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
d4f2f3f
feat(27255): allow local modification for remote feature flags
DDDDDanica Jan 14, 2025
887ed35
Update LavaMoat policies
metamaskbot Jan 14, 2025
4cef487
feat(27255): move the file to git ignore and add instructions to Read…
DDDDDanica Jan 16, 2025
5e3788b
feat(29629): Add _flags to webpack build only when in development; an…
DDDDDanica Jan 16, 2025
5c703f2
feat(29629): adapted to right pattern in transform function
DDDDDanica Jan 16, 2025
35fe1f0
feat(29629): relocate `getManifestFlags` to `shared/lib/manifestFlags`
DDDDDanica Jan 16, 2025
473a3bd
feat(29629): enrich `getRemoteFeatureFlags` return type
DDDDDanica Jan 16, 2025
caa4b4f
feat(29629): shallow merge states in `getRemoteFeatureFlags`
DDDDDanica Jan 16, 2025
0bff314
Update LavaMoat policies
metamaskbot Jan 16, 2025
d4e79a7
feat(29629): fix e2e test due to shallow merging
DDDDDanica Jan 17, 2025
8e88e7f
feat(29629): Update readme
DDDDDanica Jan 17, 2025
b1a52ae
feat(29629): Remove unused [] param in getWebpackConfig test
DDDDDanica Jan 17, 2025
b2b5862
feat(29629): Remove unnecessary --test flag in non-default options te…
DDDDDanica Jan 17, 2025
a5ea54f
feat(29629): Remove comment in json file to avoid copy error; read .m…
DDDDDanica Jan 17, 2025
5308bed
feat(29629): Move getRemoteFeatureFlags selector to a separate ts file
DDDDDanica Jan 19, 2025
2cc1560
feat(29629): Rename remote-feature-flags.ts file name
DDDDDanica Jan 19, 2025
248ec01
feat(29629): Refactor getRemoteFeatureFlags to use safe merge from lo…
DDDDDanica Jan 19, 2025
40f3c07
feat(29629): Rename gitnore comment
DDDDDanica Jan 19, 2025
3a16019
feat(29629): Remove asyn reading for loadManifestFlags in normal build
DDDDDanica Jan 19, 2025
77ad1e5
feat(29629): Fix unit test of file not exist
DDDDDanica Jan 19, 2025
b09f6a5
feat(29629): Fix lint
DDDDDanica Jan 20, 2025
9487c17
Merge branch 'main' into feature/remote-feature-flags-manifest-adaption
DDDDDanica Jan 26, 2025
51b6de6
fix(3742): fix lint
DDDDDanica Jan 26, 2025
f3c8e38
feat(27255): import json file from `.metamaskrc` instead of using rea…
DDDDDanica Jan 31, 2025
2c5ab32
feat(27255): fix lint
DDDDDanica Jan 31, 2025
945c42a
feat(27255): removed console log in build console
DDDDDanica Jan 31, 2025
f170665
Merge branch 'main' into feature/remote-feature-flags-manifest-adaption
DDDDDanica Feb 3, 2025
8be5eb3
feat(27255): update README.md
DDDDDanica Feb 3, 2025
f2cee6a
feat(27255): add the * wildcard to ignored filed name
DDDDDanica Feb 3, 2025
3a53d83
feat(27255): fix lint and try to fix "TypeError: Cannot destructure p…
DDDDDanica Feb 3, 2025
a721328
Merge branch 'main' into feature/remote-feature-flags-manifest-adaption
DDDDDanica Feb 11, 2025
ff643b6
Simplify remoteFeatureFlag type in manifestFlags.ts file
DDDDDanica Feb 11, 2025
af74af6
Modify plugins.ManifestPlugin.test.ts to use mock utility from node:test
DDDDDanica Feb 11, 2025
7c3ff23
Add silently ignores non-ENOENT filesystem unit test
DDDDDanica Feb 11, 2025
ab7448d
Make manifest.overrides.json more general
DDDDDanica Feb 12, 2025
581831c
Fix lint; revert to `fs.readFile`
DDDDDanica Feb 12, 2025
b120921
Merge branch 'main' into feature/remote-feature-flags-manifest-adaption
HowardBraham Feb 12, 2025
e950aa3
Fix failed unit tests
DDDDDanica Feb 13, 2025
b801c6c
Merge branch 'main' into feature/remote-feature-flags-manifest-adaption
DDDDDanica Feb 13, 2025
84bc8f1
Add more unit tests for overwrite manifest case
DDDDDanica Feb 13, 2025
86211f3
Merge remote-tracking branch 'origin/feature/remote-feature-flags-man…
DDDDDanica Feb 13, 2025
b768704
Changed copy of develop options from designer
DDDDDanica Feb 19, 2025
1003d35
Merge branch 'main' into feature/remote-feature-flags-manifest-adaption
DDDDDanica Feb 19, 2025
4248edf
Update snapshot
DDDDDanica Feb 19, 2025
a199f6f
refactor to prevent manifest transforms from modifying original object
DDDDDanica Feb 19, 2025
6e112e8
fix lint for return in the function
DDDDDanica Feb 19, 2025
1071e2b
change develop options context
DDDDDanica Feb 20, 2025
dcb78e3
fix snpapshot
DDDDDanica Feb 20, 2025
96fe46d
Fix lint
DDDDDanica Feb 20, 2025
9f92cc9
Improve word and lint fix for developer options
DDDDDanica Feb 20, 2025
49542c0
Rollback manifestClone copy method
DDDDDanica Feb 20, 2025
0380cf0
Revert addTabsPermission
DDDDDanica Feb 20, 2025
d8fb679
Restore mock after each test
DDDDDanica Feb 20, 2025
5ad5fdb
Update snapshot
DDDDDanica Feb 20, 2025
1e89e33
Update readme
DDDDDanica Feb 20, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
feat(29629): Add _flags to webpack build only when in development; an…
…d restore default value for webpack config without passing in `--test`
DDDDDanica committed Jan 16, 2025
commit 5e3788b55a730488932bfe33107251729bed12b8
2 changes: 1 addition & 1 deletion development/webpack/test/plugins.ManifestPlugin.test.ts
Original file line number Diff line number Diff line change
@@ -232,7 +232,7 @@ describe('ManifestPlugin', () => {
function runTest(baseManifest: Combination<typeof manifestMatrix>) {
const manifest = baseManifest as unknown as chrome.runtime.Manifest;
const hasTabsPermission = (manifest.permissions || []).includes('tabs');
const transform = transformManifest(args);
const transform = transformManifest(args, false);

if (args.test && hasTabsPermission) {
it("throws in test mode when manifest already contains 'tabs' permission", () => {
6 changes: 4 additions & 2 deletions development/webpack/test/webpack.config.test.ts
Original file line number Diff line number Diff line change
@@ -78,7 +78,7 @@ ${Object.entries(env)
}

it('should have the correct defaults', () => {
const config: Configuration = getWebpackConfig(['--test']);
const config: Configuration = getWebpackConfig([]);
// check that options are valid
const { options } = webpack(config);
assert.strictEqual(options.name, 'MetaMask – development');
@@ -162,8 +162,10 @@ ${Object.entries(env)
{
manifest_version: 3,
name: 'name',
permissions: ['tabs'],
version: '1.2.3',
_flags: {
remoteFeatureFlags: {},
},
content_scripts: [
{
js: ['scripts/contentscript.js', 'scripts/inpage.js'],
10 changes: 7 additions & 3 deletions development/webpack/utils/plugins/ManifestPlugin/helpers.ts
Original file line number Diff line number Diff line change
@@ -16,11 +16,15 @@ try {
* @param args
* @param args.lockdown
* @param args.test
* @param isDevelopment
* @returns a function that will transform the manifest JSON object
* @throws an error if the manifest already contains the "tabs" permission and
* `test` is `true`
*/
export function transformManifest(args: { lockdown: boolean; test: boolean }) {
export function transformManifest(
args: { lockdown: boolean; test: boolean },
isDevelopment: boolean,
) {
const transforms: ((manifest: chrome.runtime.Manifest) => void)[] = [];

function removeLockdown(browserManifest: chrome.runtime.Manifest) {
@@ -40,8 +44,8 @@ export function transformManifest(args: { lockdown: boolean; test: boolean }) {
browserManifest._flags = manifestFlags;
}

// Add manifest flags only for non-test builds so the test build is not affected by local feature flags
if (!args.test) {
// Add manifest flags only for development builds
if (isDevelopment) {
transforms.push(addManifestFlags);
}

2 changes: 1 addition & 1 deletion development/webpack/webpack.config.ts
Original file line number Diff line number Diff line change
@@ -131,7 +131,7 @@ const plugins: WebpackPluginInstance[] = [
version: version.version,
versionName: version.versionName,
browsers: args.browser,
transform: transformManifest(args),
transform: transformManifest(args, isDevelopment),
zip: args.zip,
...(args.zip
? {

Unchanged files with check annotations Beta

// TODO: Re think how to test this without exposing internal state
// it('should replace ethers instance when called with a different chainId than was current when the controller was instantiated', async function () {

Check warning on line 1265 in app/scripts/controllers/swaps/swaps.test.ts

GitHub Actions / Test lint / Test lint

Some tests seem to be commented
// fetchTradesInfoStub.mockReset();
// const _swapsController = getSwapsController();
// expect(currentEthersInstance).not.toStrictEqual(newEthersInstance);
// });
// it('should not replace ethers instance when called with the same chainId that was current when the controller was instantiated', async function () {

Check warning on line 1286 in app/scripts/controllers/swaps/swaps.test.ts

GitHub Actions / Test lint / Test lint

Some tests seem to be commented
// const _swapsController = new SwapsController({
// getBufferedGasLimit: MOCK_GET_BUFFERED_GAS_LIMIT,
// provider,
// expect(currentEthersInstance).toStrictEqual(newEthersInstance);
// });
// it('should replace ethers instance, and _ethersProviderChainId, twice when called twice with two different chainIds, and successfully set the _ethersProviderChainId when returning to the original chain', async function () {

Check warning on line 1306 in app/scripts/controllers/swaps/swaps.test.ts

GitHub Actions / Test lint / Test lint

Some tests seem to be commented
// const _swapsController = new SwapsController({
// getBufferedGasLimit: MOCK_GET_BUFFERED_GAS_LIMIT,
// provider,
});
});
// it('clears polling timeout', function () {

Check warning on line 1390 in app/scripts/controllers/swaps/swaps.test.ts

GitHub Actions / Test lint / Test lint

Some tests seem to be commented
// swapsController._pollingTimeout = setTimeout(() => {
// throw new Error('Polling timeout not cleared');
// }, POLLING_TIMEOUT);
describe('stopPollingForQuotes', function () {
// TODO: Re think how to test this without exposing internal state
// it('clears polling timeout', function () {

Check warning on line 1406 in app/scripts/controllers/swaps/swaps.test.ts

GitHub Actions / Test lint / Test lint

Some tests seem to be commented
// swapsController._pollingTimeout = setTimeout(() => {
// throw new Error('Polling timeout not cleared');
// }, POLLING_TIMEOUT);
describe('resetPostFetchState', function () {
// TODO: Re think how to test this without exposing internal state
// it('clears polling timeout', function () {

Check warning on line 1429 in app/scripts/controllers/swaps/swaps.test.ts

GitHub Actions / Test lint / Test lint

Some tests seem to be commented
// swapsController._pollingTimeout = setTimeout(() => {
// throw new Error('Polling timeout not cleared');
// }, POLLING_TIMEOUT);
};
setTokensListDetected(newTokensList());
}, [

Check warning on line 117 in ui/components/app/detected-token/detected-token.js

GitHub Actions / Test lint / Test lint

React Hook useEffect has a missing dependency: 'tokensListDetected'. Either include it or remove the dependency array
isTokenNetworkFilterEqualCurrentNetwork,
detectedTokensMultichain,
detectedTokens,
* No name has been saved for the value and type.
*/
export const NoSavedName = {
name: 'No Saved Name',

Check warning on line 129 in ui/components/app/name/name.stories.tsx

GitHub Actions / Test lint / Test lint

Named exports should not use the name annotation if it is redundant to the name that would be generated by the export name
args: {
value: ADDRESS_MOCK,
type: NameType.ETHEREUM_ADDRESS,
useEffect(() => {
return () => interfaceId && dispatch(deleteInterface(interfaceId));
}, [interfaceId]);

Check warning on line 49 in ui/components/app/snaps/snap-home-page/snap-home-renderer.js

GitHub Actions / Test lint / Test lint

React Hook useEffect has a missing dependency: 'dispatch'. Either include it or remove the dependency array
useEffect(() => {
// Snaps are allowed to redirect to their own pending confirmations (templated or not)
} else if (snapApproval) {
history.push(`${CONFIRM_TRANSACTION_ROUTE}/${snapApproval.id}`);
}
}, [unapprovedTemplatedConfirmations, unapprovedConfirmations, history]);

Check warning on line 65 in ui/components/app/snaps/snap-home-page/snap-home-renderer.js

GitHub Actions / Test lint / Test lint

React Hook useEffect has a missing dependency: 'snapId'. Either include it or remove the dependency array
if (error) {
return (