fix(babel): move Babel dependencies to (optional) peerDependencies #1150

Kocal · 2022-08-31T14:25:16Z

This PR does the following things:

BC: remove unecessary Babel plugins dependencies, which are already managed by @babel/present-env
move @babel/core and @babel/preset-env as required peer dependencies.
added a bunch of testing apps, to check how Encore installs/runs when:
- Babel is minimally present (just @babel/core and @babel/preset-env are installed)
- Babel is configured through Encore*, with @babel/preset-env installed and a Babel plugin not managed by @babel/preset-env (e.g.: @babel/plugin-proposal-partial-application)
- Babel is configured through babel.config.js, with @babel/preset-env installed and a Babel plugin not managed by @babel/preset-env (e.g.: @babel/plugin-proposal-partial-application)

I will update the Symfony recipe after merging this PR.

Note: in the future, I would like to make Babel totally optional with Encore, because I would like to use ESBuild for building JS/TS files (with esbuild-loader) and minifying file.

stof · 2022-08-31T15:36:25Z

We should check for their presence when you enable babel. IIRC, they are currently not checked as they were installed as dependencies of Encore.

package.json

weaverryan

Thanks for more great work on this - just one question, again, about peer dependencies, optional vs required :)

weaverryan · 2022-09-01T14:06:08Z

package.json

@@ -135,12 +134,15 @@
    "webpack-notifier": "^1.15.0"
  },
  "peerDependenciesMeta": {
-    "@babel/plugin-proposal-class-properties": {
+    "@babel/core": {
      "optional": true
    },


Are this and @babel/preset-env truly optional? Doesn't encore ALWAYS include the babel loader?

webpack-encore/lib/config-generator.js

Line 313 in 417981c

use: babelLoaderUtil.getLoaders(this.webpackConfig)

And what about the babel-loader package itself - shouldn't that also be a (required) peer dependency?

babel-loader is resolved with require.resolve() inside Encore, so I think a normal dependency is fine.

Btw, maybe @babel/preset-env could stay a normal dependency too.

And yes, I think @babel/core is a mandatory dependency as we don't allow disabling babel.

@babel/core is used at the very start when running Encore, to check if a external configuration already exists or not, throwing this error when @babel/core is not installed:

/home/runner/work/webpack-encore/webpack-encore/test_apps/yarn-pnp/.pnp.cjs:14994 Error.captureStackTrace(firstError); ^ Error: @symfony/webpack-encore tried to access @babel/core (a peer dependency) but it isn't provided by your application; this makes the require call ambiguous and unsound. Required package: @babel/core Required by: @symfony/webpack-encore@virtual:dc3fc578bfa5e06182a4d2be[39](https://github.com/symfony/webpack-encore/runs/8154620226?check_suite_focus=true#step:6:46)ede0bc5b749[40](https://github.com/symfony/webpack-encore/runs/8154620226?check_suite_focus=true#step:6:47)b1ffe0d70c26892ab140a4699787750fba175dc306292e80b4aa2c8c5f68c2a821e69b2c37e360c0dff36ff66#file:../../webpack-encore.tgz::locator=root-workspace-0b6124%40workspace%3A. (via /home/runner/work/webpack-encore/webpack-encore/test_apps/yarn-pnp/.yarn/__virtual__/@symfony-webpack-encore-virtual-7caffb10c5/0/cache/@symfony-webpack-encore-file-bd170dab52-fd868e14e8.zip/node_modules/@symfony/webpack-encore/lib/config/) Ancestor breaking the chain: root-workspace-0b6124@workspace:. Require stack: - /home/runner/work/webpack-encore/webpack-encore/test_apps/yarn-pnp/.yarn/__virtual__/@symfony-webpack-encore-virtual-7caffb10c5/0/cache/@symfony-webpack-encore-file-bd170dab52-fd868e14e8.zip/node_modules/@symfony/webpack-encore/lib/config/parse-runtime.js - /home/runner/work/webpack-encore/webpack-encore/test_apps/yarn-pnp/.yarn/__virtual__/@symfony-webpack-encore-virtual-7caffb10c5/0/cache/@symfony-webpack-encore-file-bd170dab52-fd868e14e8.zip/node_modules/@symfony/webpack-encore/bin/encore.js at Function.require$$0.Module._resolveFilename (/home/runner/work/webpack-encore/webpack-encore/test_apps/yarn-pnp/.pnp.cjs:14994:13) at Function.require$$0.Module._load (/home/runner/work/webpack-encore/webpack-encore/test_apps/yarn-pnp/.pnp.cjs:14848:[42](https://github.com/symfony/webpack-encore/runs/8154620226?check_suite_focus=true#step:6:49)) at Module.require (internal/modules/cjs/loader.js:974:19) at require (internal/modules/cjs/helpers.js:101:18) at Object.<anonymous> (/home/runner/work/webpack-encore/webpack-encore/test_apps/yarn-pnp/.yarn/__virtual__/@symfony-webpack-encore-virtual-7caffb10c5/0/cache/@symfony-webpack-encore-file-bd170dab52-fd868e14e8.zip/node_modules/@symfony/webpack-encore/lib/config/parse-runtime.js:15:15) at Module._compile (internal/modules/cjs/loader.js:1085:14) at Object.Module._extensions..js (internal/modules/cjs/loader.js:1114:10) at Object.require$$0.Module._extensions..js (/home/runner/work/webpack-encore/webpack-encore/test_apps/yarn-pnp/.pnp.cjs:15038:33) at Module.load (internal/modules/cjs/loader.js:950:32) at Function.require$$0.Module._load (/home/runner/work/webpack-encore/webpack-encore/test_apps/yarn-pnp/.pnp.cjs:1[48](https://github.com/symfony/webpack-encore/runs/8154620226?check_suite_focus=true#step:6:55)78:14)

And then @babel/preset-env is required when configuring Babel through Encore, if no Babel external configuration (through a file) is found.

I will make it non-optional, but in the future I would like to be able to make Babel completely optional (if people wants to use ESbuild instead, for better performances).

I will make it non-optional, but in the future I would like to be able to make Babel completely optional (if people wants to use ESbuild instead, for better performances).

Yup, I agree. But I also agree that we should get this big chore done first, then do that. 👍

Kocal · 2022-09-02T11:50:56Z

.github/workflows/testing_apps.yml

+          - name: npm (with Babel)
+            working_directory: test_apps/npm-with-babel
+            script: |
+              npm install --ci
+              npm add --save-dev ../../webpack-encore.tgz
+              npm run encore dev
+              npm run encore production
+
+          - name: npm (with external Babel configuration file)
+            working_directory: test_apps/npm-with-external-babel-config
+            script: |
+              npm install --ci
+              npm add --save-dev ../../webpack-encore.tgz
+              npm run encore dev
+              npm run encore production
+


Sadly there is no way with GitHub Actions to use Yaml anchors, so we had to duplicate the script each time. 😕

Kocal · 2022-09-02T11:51:41Z

lib/loaders/babel.js

@@ -55,14 +55,13 @@ module.exports = {
                presets: [
                    [require.resolve('@babel/preset-env'), presetEnvOptions]
                ],
-                plugins: [require.resolve('@babel/plugin-syntax-dynamic-import')]
+                plugins: []


I keep it even if empty, in order to prevent more breaking changes, if people used config.plugins.push() like we did in tests.

weaverryan · 2022-09-08T00:38:17Z

Thank you Hugo!

weaverryan · 2022-09-08T01:03:06Z

Btw, my testing locally seems to all be working now! The recipe needs a few updates - symfony/recipes#1123 - and we'll also need a 4.0.0 entry in the CHANGELOG. If you want to add that, it would be much appreciated :).

This PR was merged into the main branch. Discussion ---------- chore: add changelog for 4.0.0 As asked in #1150 (comment). Commits ------- b60db90 chore: add changelog for 4.0.0

Kocal force-pushed the fix/babel-with-pnp branch 2 times, most recently from f916074 to 27ec15a Compare August 31, 2022 14:31

Kocal mentioned this pull request Aug 31, 2022

Error using Encore 4 + yarn pnp: cannot find package from Babel #1149

Closed

stof requested changes Aug 31, 2022

View reviewed changes

package.json Outdated Show resolved Hide resolved

package.json Outdated Show resolved Hide resolved

Kocal force-pushed the fix/babel-with-pnp branch 3 times, most recently from 0048711 to 731c49c Compare August 31, 2022 19:43

Kocal requested a review from stof August 31, 2022 19:44

weaverryan reviewed Sep 1, 2022

View reviewed changes

Kocal force-pushed the fix/babel-with-pnp branch 2 times, most recently from 9494e77 to 41ef3a5 Compare September 2, 2022 10:56

Kocal commented Sep 2, 2022

View reviewed changes

Kocal requested review from weaverryan and stof and removed request for stof and weaverryan September 2, 2022 11:56

weaverryan approved these changes Sep 8, 2022

View reviewed changes

fix(babel): move Babel dependencies to (optional) peerDependencies

418042f

weaverryan force-pushed the fix/babel-with-pnp branch from 5582e68 to 418042f Compare September 8, 2022 00:38

weaverryan merged commit 79fb073 into symfony:main Sep 8, 2022

Kocal deleted the fix/babel-with-pnp branch September 8, 2022 04:59

Kocal mentioned this pull request Sep 8, 2022

chore: add changelog for 4.0.0 #1152

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(babel): move Babel dependencies to (optional) peerDependencies #1150

fix(babel): move Babel dependencies to (optional) peerDependencies #1150

Kocal commented Aug 31, 2022 •

edited

Loading

stof commented Aug 31, 2022

weaverryan left a comment

weaverryan Sep 1, 2022

stof Sep 1, 2022

Kocal Sep 2, 2022 •

edited

Loading

weaverryan Sep 8, 2022

Kocal Sep 2, 2022

Kocal Sep 2, 2022

weaverryan commented Sep 8, 2022 •

edited

Loading

weaverryan commented Sep 8, 2022

fix(babel): move Babel dependencies to (optional) peerDependencies #1150

fix(babel): move Babel dependencies to (optional) peerDependencies #1150

Conversation

Kocal commented Aug 31, 2022 • edited Loading

stof commented Aug 31, 2022

weaverryan left a comment

Choose a reason for hiding this comment

weaverryan Sep 1, 2022

Choose a reason for hiding this comment

stof Sep 1, 2022

Choose a reason for hiding this comment

Kocal Sep 2, 2022 • edited Loading

Choose a reason for hiding this comment

weaverryan Sep 8, 2022

Choose a reason for hiding this comment

Kocal Sep 2, 2022

Choose a reason for hiding this comment

Kocal Sep 2, 2022

Choose a reason for hiding this comment

weaverryan commented Sep 8, 2022 • edited Loading

weaverryan commented Sep 8, 2022

Kocal commented Aug 31, 2022 •

edited

Loading

Kocal Sep 2, 2022 •

edited

Loading

weaverryan commented Sep 8, 2022 •

edited

Loading