[App Extensibility ⚙️] Fix Babel configuration ignore `node_modules` folder (@W-17373534@) #2228

adamraya · 2025-01-29T02:18:07Z

Description

Problem

We identified an issue where Babel is transpiling all files in node_modules due to using the wrong Babel configuration, leading to two warnings when we start the template-typescript-minimal project.

...
[BABEL] Note: The code generator has deoptimised the styling of /path/to/react-dom/cjs/react-dom.development.js as it exceeds the max of 500KB
[BABEL] Note: The code generator has deoptimised the styling of /path/to/react-dom/cjs/react-dom.development.js as it exceeds the max of 500KB

Solution

The PR fixes the Babel configuration to avoid transpiling all files in node_modules, solving the two warnings we saw when starting the dev server.

The Babel warnings no longer appear when starting the dev server.

Screenshare.-.2025-01-28.6_27_27.PM.mp4

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)

Breaking changes include:

Removing a public function or component or prop

Adding a required argument to a function

Changing the data type of a function parameter or return value

Adding a new peer dependency to package.json

Changes

Updated pwa-kit-dev Webpack Babel Configuration
Updated pwa-kit-dev Babel Configuration

How to Test-Drive This PR

Run the generator with this configuration

node packages/pwa-kit-create-app/scripts/create-mobify-app-dev.js

? What type of PWA Kit project would you like to create? PWA Kit Application
? Do you want to use Application Extensibility? Yes
? Which Application Extensions do you want to install? @salesforce/extension-chakra-store-locator, @salesforce/extension-chakra-storefront
? ⚠️ WARNING: If you choose to extract the Application Extension code,
you will NO LONGER be able to consume upgrades from NPM. All changes
made to the extracted code will be YOUR RESPONSIBILITY.

Do you want to proceed with extracting the Application Extensions code? No
? What is the name of your Project? test
Installing dependencies for test... This may take a few minutes.

Copy the content of packages/extension-chakra-storefront/config/sample.json into the extension-chakra-storefront inside the generated project package.json file.
Strat the project removing Babel cache.

rm -Rf node_modules/.cache && BABEL_DISABLE_CACHE=1 npm start.

Verify

Observe no Babel warnings are present in the dev server logs.
Smoke test the extensions by navigating through the storefront

Checklists

General

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

Accessibility Compliance

You must check off all items in one of the follow two lists:

There are no changes to UI

or...

Changes were tested with a Screen Reader (iOS VoiceOver or Android Talkback) and had no issues
Changes comply with WCAG 2.0 guidelines levels A and AA
Changes to common UI patterns and interactions comply with WAI-ARIA best practices

Localization

Changes include a UI text update in the Retail React App (which requires translation)

)" This reverts commit 712f744

…odules

This reverts commit 712f744

kevinxh · 2025-01-29T17:45:07Z

packages/pwa-kit-dev/src/configs/babel/babel-config.js

+        function (filepath) {
+            // Ignore node_modules, but include @salesforce/extension-* and pwa-kit-extension-sdk
+            if (/node_modules/.test(filepath)) {
+                // Only ignore non-extension @salesforce packages
+                if (
+                    /node_modules\/(?!@salesforce\/pwa-kit-extension-sdk|@salesforce\/extension-)/.test(
+                        filepath
+                    )
+                ) {
+                    return true
+                }
+                return false
+            }
+            return false
+        }


I asked AI to invert the logic to get rid of the nested conditions. Here is the output i got:

function (filepath) { // Return false if it's an allowed extension package if (/node_modules\/@salesforce\/(pwa-kit-extension-sdk|extension-)/.test(filepath)) { return false; } // Return true if it's in node_modules (excluding allowed packages handled above) if (/node_modules/.test(filepath)) { return true; } // Return false for all other files return false; }

kevinxh · 2025-01-29T17:48:11Z

packages/pwa-kit-dev/src/configs/webpack/config.js

+        // By default babel doesn't process files in "node_modules" folder, so here we will ensure they are included.
+        exclude: /node_modules\/(?!(@?[^/]+\/)?extension-)[^/]+\/.*$/i,


normally I wouldn't advocate to document the "what", only the "why"s.

But I feel like when it comes to regexes, we should add a quick explanation of what this regex aims to do. Because it's hard to tell unless you copy and paste the regex to a tool like https://regexr.com/

i.e. This regex exclude everything in node_modules, but node_modules/extensions-*/ folders

alexvuong · 2025-01-29T18:33:14Z

packages/pwa-kit-dev/src/configs/babel/babel-config.js

-    }
+    },
+    ignore: [
+        function (filepath) {


TIL you can pass a function into babel config ignore :-o. Any doc link about this?

It's basically creating an inline Babel plugin. See https://babeljs.io/docs/plugins#plugin-development

alexvuong · 2025-01-29T18:38:38Z

packages/pwa-kit-dev/src/configs/babel/babel-config.js

+                // Only ignore non-extension @salesforce packages
+                if (
+                    /node_modules\/(?!@salesforce\/pwa-kit-extension-sdk|@salesforce\/extension-)/.test(
+                        filepath


Unblocking comment, let's look a bit further in the future when partners start to create their own extensions. Do they have to follow the pattern of @salesforce/extension-* or just extension-* or their brand name like this @company/extension?
If it is more than @salesforce/extension-* we may want to allow other pattern here too to avoid bug later down the road.

alexvuong · 2025-01-29T18:42:28Z

packages/pwa-kit-dev/src/configs/webpack/config.js

@@ -340,6 +340,8 @@ const ruleForBabelLoader = (babelPlugins) => {
        id: 'babel-loader',
        test: /(\.js(x?)|\.ts(x?))$/,
        // NOTE: Because our extensions are just folders containing source code, we need to ensure that the babel-loader processes them.
+        // By default babel doesn't process files in "node_modules" folder, so here we will ensure they are included.
+        exclude: /node_modules\/(?!(@?[^/]+\/)?extension-)[^/]+\/.*$/i,


Dumb question, we have exclude here and ignore in the babel config? How are they working with each other? I recall they can't be used on a same directory like node_modules because exclude is exclusive to the folder and will override ignore.

These are different configurations that don't work with each other. One is the Webpack configuration for babel-loader, and the other is the Babel configuration for babel-node, where we cannot use the exclude option. They are used in separate stages.

alexvuong

Tested locally. Verified there were no Babel warnings in the terminal. LGTM

adamraya added 10 commits January 23, 2025 16:59

Babel ignore node_modules files

b9f3a7e

Revert "[App Extensibility ⚙️] Fix Slow Dev Server (@W-17131556@) (#2115

2230d17

)" This reverts commit 712f744

WIP Add debug logs

0401f10

Merge branch 'feature/extensibility-v2' into W-17373534-ignore-node_m…

c099e68

…odules

WIP babel-config only

d5de6f6

This reverts commit 712f744

WIP restore pwa-kit-dev start command

049e826

WIP babel-config ignore

9a491a9

Clean up

9e3d3b8

Fix Babel config

2f1c77b

Clean up

90d0357

adamraya added the skip changelog Skip the "Changelog Check" GitHub Actions step even if the Changelog.md files are not updated label Jan 29, 2025

adamraya requested a review from a team as a code owner January 29, 2025 02:18

adamraya added 2 commits January 28, 2025 18:34

lint

bf4968d

Remove Babel ignore flag

394f9b3

kevinxh reviewed Jan 29, 2025

View reviewed changes

kevinxh approved these changes Jan 29, 2025

View reviewed changes

alexvuong reviewed Jan 29, 2025

View reviewed changes

adamraya added 2 commits January 29, 2025 15:40

PR Feedback

e60e441

Clean up testing code

64e45c7

alexvuong approved these changes Jan 30, 2025

View reviewed changes

Re-add tests

645f65d

adamraya merged commit 144759b into feature/extensibility-v2 Jan 30, 2025
27 checks passed

adamraya deleted the W-17373534-ignore-node_modules branch January 30, 2025 01:35

adamraya mentioned this pull request Jan 30, 2025

[App Extensibility ⚙️] Fix Babel configuration ignore node_modules folder adjusted for Windows (@W-17373534@) #2230

Merged

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[App Extensibility ⚙️] Fix Babel configuration ignore `node_modules` folder (@W-17373534@) #2228

[App Extensibility ⚙️] Fix Babel configuration ignore `node_modules` folder (@W-17373534@) #2228

adamraya commented Jan 29, 2025 •

edited

Loading

kevinxh Jan 29, 2025

kevinxh Jan 29, 2025

kevinxh Jan 29, 2025

alexvuong Jan 29, 2025

adamraya Jan 30, 2025

alexvuong Jan 29, 2025

alexvuong Jan 29, 2025

adamraya Jan 30, 2025

alexvuong left a comment

		// By default babel doesn't process files in "node_modules" folder, so here we will ensure they are included.
		exclude: /node_modules\/(?!(@?[^/]+\/)?extension-)[^/]+\/.*$/i,

[App Extensibility ⚙️] Fix Babel configuration ignore node_modules folder (@W-17373534@) #2228

[App Extensibility ⚙️] Fix Babel configuration ignore node_modules folder (@W-17373534@) #2228

Conversation

adamraya commented Jan 29, 2025 • edited Loading

Description

Problem

Solution

Types of Changes

Changes

How to Test-Drive This PR

Verify

Checklists

General

Accessibility Compliance

Localization

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexvuong left a comment

Choose a reason for hiding this comment

[App Extensibility ⚙️] Fix Babel configuration ignore `node_modules` folder (@W-17373534@) #2228

[App Extensibility ⚙️] Fix Babel configuration ignore `node_modules` folder (@W-17373534@) #2228

adamraya commented Jan 29, 2025 •

edited

Loading