Skip to content

Allow Extensions to use .force_overrides @@W-18216861@@ #2380

Merged
bendvc merged 10 commits intofeature/extensibility-v2from
bendvc/W-18216861_extension-force-overrides
May 6, 2025
Merged

Allow Extensions to use .force_overrides @@W-18216861@@ #2380
bendvc merged 10 commits intofeature/extensibility-v2from
bendvc/W-18216861_extension-force-overrides

Conversation

@bendvc
Copy link
Contributor

@bendvc bendvc commented Apr 29, 2025

Description

As we make the transition from our current version of extensibility (v3) to the next version (v4) we believe that customers will want to retain many of their overrides that we currently have. This lead to the birth of the .force_overrides file which allows you to overrides an extensions public file API and provide overrides for any file that exists in an extension. Allowing the customer to unwind their level of overrides to zero if possible to get them to a place where upgradability isn't a challenge.

Currently this feature only works in the base project (aka the project that you generate) but not in an extension project. This has been identified as a short coming of our API via one of our feedback sessions where a users voiced that they might want to override more than the public API if required. This could be for scenarios where they don't want to wait for use to make changes to our extensions to support features or customizations they want.

In this PR I enable the .force_overrides for application extension projects.

Types of Changes

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Documentation update
  • Breaking change (could cause existing functionality to not work as expected)
  • Other changes (non-breaking changes that does not fit any of the above)

Changes

  • OverridesResolverLoader uses a combined list of all "force_override" file paths in the base project and the extensions.

How to Test-Drive This PR

I have left in the template-typescript-minimal project configured with the store locator and the starter extensions. The starter extension has been updated to have a force_overrides file and an override to the store locator that isn't normally overridable.

  • In the template-typescript-minimal project run npm run start
  • Once the application is up and running, verify that the override added to the starter extension works. You can do this by going to the /store-locator path.
  • You should see a page that have a store locator content component that simply says hello

Checklists

General

  • Changes are covered by test cases
  • CHANGELOG.md updated with a short description of changes (not required for documentation updates)

TODO

  • Revert changes to typescript minimal configuration.

@bendvc bendvc requested a review from a team as a code owner April 29, 2025 17:13
@bendvc bendvc requested review from adamraya and vmarta April 29, 2025 17:28
@bendvc bendvc added the skip changelog Skip the "Changelog Check" GitHub Actions step even if the Changelog.md files are not updated label Apr 29, 2025
@cc-prodsec
Copy link
Collaborator

cc-prodsec commented Apr 29, 2025

🎉 Snyk checks have passed. No issues have been found so far.

security/snyk check is complete. No issues have been found. (View Details)

license/snyk check is complete. No issues have been found. (View Details)

Copy link
Contributor

@vmarta vmarta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've test-driven the PR and could see the app extension doing a force override successfully as described.

// Lets use the compiler configuration to ensure we are resolving the correct file extensions.
const compilerOptions = this._compiler?.options
const extensions = compilerOptions?.resolve?.extensions || []
const extensions = options?.resolveExtensions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: this is where the webpack world and our app extensions collide :) Maybe rename this extensions to fileExtensions to differentiate it from app extensions?

}

/**
* PRIVATE: Reads and parses a .force_overrides file into a list of clean override entries.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: jsdoc has @private that we can use here.

const projectPath = resourcePath.split(SRC)[0]
const projectRelPath = resourcePath.split(`${SRC_FOLDER}${path.sep}`)[1].split('.')[0] // File path relative to the project directory without file extension
const projectPath = resourcePath.split(SRC_FOLDER)[0]
const options = this.getOptions()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we print a warning for visibility when we detect the .force_overrides is being used with a similar copy?

// BY USING THIS FILE, YOU AGREE THAT THE FUNCTIONALITY OF YOUR INSTALLED EXTENSION(S) IS NOT GUARANTEED.
// ADDITIONALLY UPGRADABILITY OF EXTENSIONS CAN ALSO NO LONGER BE GUARANTEED AND IS NOT SUPPORTED BY SALESFORCE.
// USE ONLY AS A TEMPORARY SOLUTION TO URGENTLY PATCH/UPDATE AN EXTENSION.

Force overrides are in use. This may impact the upgradeability of extensions. Use only as a temporary solution for urgent fixes.

Copy link
Contributor

@adamraya adamraya May 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could print the warning based on the overridablesSet:

if (overridablesSet.size > 0) {
        showWarning()
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We decided for now not to do this as we are unsure of the path forward on force_overrides.

Copy link
Contributor

@adamraya adamraya May 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to update the script npm run list-overrides to list the unused file override defined in the extensions .force-overrides?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out that this script doesn't use the force-overrides directly. So we are going to create a new ticket for this once we get a better idea on the scope of the requirements.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines +103 to +108
return content
.split(/\r?\n/)
.map((line) => line.trim())
.filter((line) => line && !line.startsWith('//'))
.map((line) => line.split('//')[0].trim()) // remove inline comments
.filter(Boolean)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The .filter(Boolean) at the end looks like is redundant

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is a line with only a comment on it, this filter ensures that the empty line isn't included.

return [
path.join(projectDir, OVERRIDABLE_FILE_NAME),
...extensions
.map((ext) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we can use flatMap() instead of .map() ... .flat()

// MULTIPLE OVERRIDES CAN BE ADDED TO THIS FILE, ONE PER LINE.\
// EXAMPLE:
// ./node_modules/@salesforce/extension-sample/src/pages/home.tsx No newline at end of file
./node_modules/@salesforce/extension-sample/src/pages/home.tsx No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you want to revert this change by default.

@bendvc bendvc merged commit 83c0a71 into feature/extensibility-v2 May 6, 2025
33 checks passed
@bendvc bendvc deleted the bendvc/W-18216861_extension-force-overrides branch July 16, 2025 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip changelog Skip the "Changelog Check" GitHub Actions step even if the Changelog.md files are not updated

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants