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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Breaking change in a minor version bump #15679

Open
1 task
schneems opened this issue Jun 2, 2023 · 10 comments
Open
1 task

[Bug]: Breaking change in a minor version bump #15679

schneems opened this issue Jun 2, 2023 · 10 comments

Comments

@schneems
Copy link

schneems commented Jun 2, 2023

馃捇

  • Would you like to work on a fix?

How are you using Babel?

babel-loader (webpack)

Input code

// Anything, the version release caused it to break old `babel.config.js` files

Configuration file name

babel.config.js

Configuration

module.exports = function(api) {
  var validEnv = ['development', 'test', 'production']
  var currentEnv = api.env()
  var isDevelopmentEnv = api.env('development')
  var isProductionEnv = api.env('production')
  var isTestEnv = api.env('test')

  if (!validEnv.includes(currentEnv)) {
    throw new Error(
      'Please specify a valid `NODE_ENV` or ' +
        '`BABEL_ENV` environment variables. Valid values are "development", ' +
        '"test", and "production". Instead, received: ' +
        JSON.stringify(currentEnv) +
        '.'
    )
  }

  return {
    presets: [
      isTestEnv && [
        '@babel/preset-env',
        {
          targets: {
            node: 'current'
          }
        }
      ],
      (isProductionEnv || isDevelopmentEnv) && [
        '@babel/preset-env',
        {
          forceAllTransforms: true,
          useBuiltIns: 'entry',
          corejs: 3,
          modules: false,
          exclude: ['transform-typeof-symbol']
        }
      ]
    ].filter(Boolean),
    plugins: [
      'babel-plugin-macros',
      '@babel/plugin-syntax-dynamic-import',
      isTestEnv && 'babel-plugin-dynamic-import-node',
      '@babel/plugin-transform-destructuring',
      [
        '@babel/plugin-proposal-class-properties',
        {
          loose: true
        }
      ],
      [
        '@babel/plugin-proposal-object-rest-spread',
        {
          useBuiltIns: true
        }
      ],
      [
        '@babel/plugin-proposal-private-methods',
        {
          loose: true
        }
      ],
      [
        '@babel/plugin-proposal-private-property-in-object',
        {
          loose: true
        }
      ],
      [
        '@babel/plugin-transform-runtime',
        {
          helpers: false
        }
      ],
      [
        '@babel/plugin-transform-regenerator',
        {
          async: false
        }
      ]
    ].filter(Boolean)
  }
}

Current and expected behavior

Before the 7.22 release projects that are locked to 7.x with the above config would execute fine. After 7.22.0 the get this error:

Compiling...
Compilation failed:
node:internal/process/promises:289
            triggerUncaughtException(err, true /* fromPromise */);
            ^

Error: Cannot find package '@babel/plugin-proposal-private-methods' imported from /private/tmp/79de2b03fd57b181232892d89e7e9920/myapp/babel-virtual-resolve-base.js
    at new NodeError (/private/tmp/79de2b03fd57b181232892d89e7e9920/myapp/node_modules/@babel/core/lib/vendor/import-meta-resolve.js:203:5)
    at packageResolve (/private/tmp/79de2b03fd57b181232892d89e7e9920/myapp/node_modules/@babel/core/lib/vendor/import-meta-resolve.js:873:9)
    at moduleResolve (/private/tmp/79de2b03fd57b181232892d89e7e9920/myapp/node_modules/@babel/core/lib/vendor/import-meta-resolve.js:902:20)
    at defaultResolve (/private/tmp/79de2b03fd57b181232892d89e7e9920/myapp/node_modules/@babel/core/lib/vendor/import-meta-resolve.js:985:15)
    at resolve (/private/tmp/79de2b03fd57b181232892d89e7e9920/myapp/node_modules/@babel/core/lib/vendor/import-meta-resolve.js:999:12)
    at resolve (/private/tmp/79de2b03fd57b181232892d89e7e9920/myapp/node_modules/@babel/core/lib/config/files/import-meta-resolve.js:13:10)
    at tryImportMetaResolve (/private/tmp/79de2b03fd57b181232892d89e7e9920/myapp/node_modules/@babel/core/lib/config/files/plugins.js:123:45)
    at resolveStandardizedNameForImport (/private/tmp/79de2b03fd57b181232892d89e7e9920/myapp/node_modules/@babel/core/lib/config/files/plugins.js:145:19)
    at resolveStandardizedName (/private/tmp/79de2b03fd57b181232892d89e7e9920/myapp/node_modules/@babel/core/lib/config/files/plugins.js:154:12)
    at loadPlugin (/private/tmp/79de2b03fd57b181232892d89e7e9920/myapp/node_modules/@babel/core/lib/config/files/plugins.js:47:20)
    at loadPlugin.next (<anonymous>)
    at createDescriptor (/private/tmp/79de2b03fd57b181232892d89e7e9920/myapp/node_modules/@babel/core/lib/config/config-descriptors.js:139:16)
    at createDescriptor.next (<anonymous>)
    at step (/private/tmp/79de2b03fd57b181232892d89e7e9920/myapp/node_modules/gensync/index.js:261:32)
    at evaluateAsync (/private/tmp/79de2b03fd57b181232892d89e7e9920/myapp/node_modules/gensync/index.js:291:5)
    at /private/tmp/79de2b03fd57b181232892d89e7e9920/myapp/node_modules/gensync/index.js:44:11
    at Array.forEach (<anonymous>)
    at Function.async (/private/tmp/79de2b03fd57b181232892d89e7e9920/myapp/node_modules/gensync/index.js:43:15)
    at Function.all (/private/tmp/79de2b03fd57b181232892d89e7e9920/myapp/node_modules/gensync/index.js:216:13)
    at Generator.next (<anonymous>)
    at createDescriptors (/private/tmp/79de2b03fd57b181232892d89e7e9920/myapp/node_modules/@babel/core/lib/config/config-descriptors.js:101:41)
    at createDescriptors.next (<anonymous>)
    at createPluginDescriptors (/private/tmp/79de2b03fd57b181232892d89e7e9920/myapp/node_modules/@babel/core/lib/config/config-descriptors.js:98:17)
    at createPluginDescriptors.next (<anonymous>)
    at /private/tmp/79de2b03fd57b181232892d89e7e9920/myapp/node_modules/@babel/core/lib/gensync-utils/functional.js:21:23
    at Generator.next (<anonymous>)
    at mergeChainOpts (/private/tmp/79de2b03fd57b181232892d89e7e9920/myapp/node_modules/@babel/core/lib/config/config-chain.js:349:34)
    at mergeChainOpts.next (<anonymous>)
    at chainWalker (/private/tmp/79de2b03fd57b181232892d89e7e9920/myapp/node_modules/@babel/core/lib/config/config-chain.js:316:14)
    at chainWalker.next (<anonymous>)
    at loadFileChain (/private/tmp/79de2b03fd57b181232892d89e7e9920/myapp/node_modules/@babel/core/lib/config/config-chain.js:192:24)
    at loadFileChain.next (<anonymous>)
    at buildRootChain (/private/tmp/79de2b03fd57b181232892d89e7e9920/myapp/node_modules/@babel/core/lib/config/config-chain.js:78:27)
    at buildRootChain.next (<anonymous>)
    at loadPrivatePartialConfig (/private/tmp/79de2b03fd57b181232892d89e7e9920/myapp/node_modules/@babel/core/lib/config/partial.js:79:62)
    at loadPrivatePartialConfig.next (<anonymous>) {
  code: 'ERR_MODULE_NOT_FOUND'
}

I originally reported this in rails/rails#48372, however, Rails 6 is no longer being maintained. The reason that this broke is because it is locked to babel 7.x as long as there are no breaking changes with 7.x it will continue to work, however 7.22 .0 changed the name of a module used in babel.config.js so the old module name expected in babel.config.js is no longer installed and this causes an error.

Environment

$ npx envinfo --preset babel
Need to install the following packages:
  envinfo@7.8.1
Ok to proceed? (y) y

  System:
    OS: macOS 13.4
  Binaries:
    Node: 19.6.1 - /usr/local/bin/node
    Yarn: 1.22.17 - /usr/local/bin/yarn
    npm: 9.4.0 - /usr/local/bin/npm
  npmPackages:
    webpack: ^4.46.0 => 4.46.0

Possible solution

Treat this as a bug/regression. Rollback the naming change in a new release of 7.22.x this will un-break old applications. Then release the breaking change in a new major version (presumably 8.x).

There's nothing wrong with breaking changes (as long as they're intentional) provided that the major version is rev-d.

In the future, I recommend adopting the policy that all module renaming is a breaking change. For some minor prior art from another language, it's commonly considered a breaking change in Rust libraries to rename a module https://stackoverflow.com/a/41195476/147390.

Additional context

You're awesome, and babel is great! Thanks for all your work 馃挏

@schneems schneems changed the title [Bug]: Breaking change in a major version bump [Bug]: Breaking change in a minor version bump Jun 2, 2023
@babel-bot
Copy link
Collaborator

Hey @schneems! We really appreciate you taking the time to report an issue. The collaborators on this project attempt to help as many people as possible, but we're a limited number of volunteers, so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack community that typically always has someone willing to help. You can sign-up here for an invite.

@nicolo-ribaudo
Copy link
Member

What's happening here is that your config is depending on @babel/plugin-proposal-private-methods without declaring it in package.json. What's causing problems for you here is not that we renamed a package, but that @babel/preset-env doesn't depend anymore on @babel/plugin-proposal-private-methods.

The Rust RFC you mentioned doesn't include "renaming a crate" as a breaking change, because it's not:

  • your application depends on crate-a version 1.2.3
  • crate-a is renamed to crate-b, and published as version 1.2.4
  • your app keeps working, it will never notice the rename unless you explicitly change your Cargo.toml file, it will simply not receive the new updates

What caused problems for you is something that in Rust can never happen, because as far as I know it's impossible to depend on a crate in Rust without explicitly adding it to your Cargo.toml:

  • you depend on @babel/preset-env
  • @babel/preset-env used to depend on @babel/plugin-proposal-private-methods
  • When using npm to install packages (or Yarn in some cases), it will generate one of the two following node_modules layouts:
    node_modules/
    - @babel/preset-env/
      - node_modules/
        - @babel/plugin-proposal-private-methods/
    
    node_modules/
    - @babel/preset-env/
    - @babel/plugin-proposal-private-methods/
    
  • Your app was depending on the second layout being generated, and it was depending on @babel/plugin-proposal-private-methods without listing it in its dependencies (note: you cannot do this in Rust!). It was relying on:
    • internal implementation details of how your package manager hoists dependencies in node_modules to save disk space
    • the internal dependencies of @babel/preset-env, and (at least in the JS ecosystem) the internal dependencies of a package are never considered part of its public API

Now that @babel/preset-env doesn't depend on @babel/plugin-proposal-private-methods anymore, node_modules/@babel/plugin-proposal-private-methods is less likely to exist (you could still be lucky and have another one of your dependencies transitively depend on that package).

The workaround you found in rails/rails#48372 (comment) is a bad workaround: it still relies on a package been present in your node_modules by luck, without explicitly listing it in your dependencies in package.json.

Two good solutions are:

  1. Make sure to list in package.json all the dependencies you use, whether or not you use the new packages or the old packages in your Babel config
  2. Simplify your config, to not depend explicitly on the various plugins just to enable their loose mode, but using the top-level assumptions option instead:
   module.exports = function(api) {
  var validEnv = ['development', 'test', 'production']
  var currentEnv = api.env()
  var isDevelopmentEnv = api.env('development')
  var isProductionEnv = api.env('production')
  var isTestEnv = api.env('test')

  if (!validEnv.includes(currentEnv)) {
    throw new Error(
      'Please specify a valid `NODE_ENV` or ' +
        '`BABEL_ENV` environment variables. Valid values are "development", ' +
        '"test", and "production". Instead, received: ' +
        JSON.stringify(currentEnv) +
        '.'
    )
  }

  return {
    assumptions: {
      setPublicClassFields: true,
      privateFieldsAsProperties: true,
    },
    presets: [
      isTestEnv && [
        '@babel/preset-env',
        {
          targets: {
            node: 'current'
          }
        }
      ],
      (isProductionEnv || isDevelopmentEnv) && [
        '@babel/preset-env',
        {
          forceAllTransforms: true,
          useBuiltIns: 'entry',
          corejs: 3,
          modules: false,
          exclude: ['transform-typeof-symbol']
        }
      ]
    ].filter(Boolean),
    plugins: [
      'babel-plugin-macros',
      [
        '@babel/plugin-proposal-object-rest-spread',
        {
          useBuiltIns: true
        }
      ],
      [
        '@babel/plugin-transform-runtime',
        {
          helpers: false
        }
      ],
    ].filter(Boolean)
  }
}
  • @babel/plugin-proposal-class-properties, @babel/plugin-proposal-private-methods, and @babel/plugin-proposal-private-property-in-object's loose can be replaced by setPublicClassFields and privateFieldsAsProperties
  • the destructuring, dynamic import and @babel/plugin-transform-regenerator plugins are already included in @babel/preset-env

Additionally, I believe that @babel/plugin-transform-runtime as configured is a no-op but I'm not 100% sure about it (there must be a reason for enabling it while explicitly disabling all its behavior, but I don't understand which it could have been)

@schneems
Copy link
Author

schneems commented Jun 2, 2023

Two good solutions are

Unfortunately, we cannot fix this in webpacker or Rails 6 as both of them are no longer under development and locked to a specific major version of babel. As long as there are no breaking changes to babel, that code will continue to work.

I understand the underlying dependency chain that is causing the issue. My main argument is that a change in babel should not break code that was previously working when people depending on it were locked to a specific major.

If your stance is that the developers who wrote this code (not me) "did it wrong," I would push back by asking how could we re-design the feature so that is either:

  • Emits a warning
  • Is harder than "doing it right"
  • Not possible

That wouldn't fix this case, or any future cases as more plugins get renamed, but it might alleviate some future pain and guide people how to use the feature as you're expecting.

It would raise the question, what exactly did the developer do wrong? It sounds like you're saying they're importing a module without requiring it explicitly. I would agree that's generally not a good idea unless one of the reasons that library exists is to expose other modules. I'm not intimately familiar with this ecosystem (and not affiliated with webpacker gem at all), this npm library.

@babel/preset-env is a smart preset that allows you to use the latest JavaScript without needing to micromanage which syntax transforms (and optionally, browser polyfills) are needed by your target environment (s).

Looking at the docs:

Babel plugins - both full and shorthand names are supported, for example the following are functionally equivalent:

It looks like a feature of this library is to export plugins. If that's the case than a change in the name of available exported plugins would be a breaking change (again, this is a very weak assertion I'm not really that familiar with the ecosystem and want to lay my cards on the table).

@liuxingbaoyu
Copy link
Member

liuxingbaoyu commented Jun 2, 2023

It looks like a feature of this library is to export plugins. If that's the case than a change in the name of available exported plugins would be a breaking change (again, this is a very weak assertion I'm not really that familiar with the ecosystem and want to lay my cards on the table).

No, it only contains @babel/plugin-transform-xxx, @babel/plugin-proposal-xxx needs to be installed manually by the developer.
This is a problem of nodejs dependency layout. When package A depends on package B and package B depends on package C, package A can sometimes directly use package C.
We're really just not using a dependency anymore (renaming is equivalent to removing dependency A and adding dependency B), and obviously changes to a package's internal dependencies should not be considered breaking changes.

But I agree that the current consequences are a bit annoying, maybe we can consider keeping the old dependencies for now.

@nicolo-ribaudo
Copy link
Member

If your stance is that the developers who wrote this code (not me) "did it wrong," I would push back by asking how could we re-design the feature so that is either:

  • Emits a warning
  • Is harder than "doing it right"
  • Not possible

There are three package managers in the JavaScript ecosystem: npm, pnpm and yarn. By default, both pnpm and yarn prevent this problem:

  • pnpm generates the first node_modules layout I mentioned in [Bug]: Breaking change in a minor version bump聽#15679 (comment), so this case would have always been an error and it wouldn't have been possible to accidentally rely on it
  • yarn (since version 2) doesn't use node_modules by default, and makes sure that every package can only depend on the packages it explicitly lists in its dependencies. Yarn 1 didn't support a way to catch this problem.
  • npm has an accepted RFC for an opt-in way to catch these problems, but:
    • it's opt-in, so it doesn't catch the problem by default
    • as far as I know, it has not been actually developed yet

I would agree that's generally not a good idea unless one of the reasons that library exists is to expose other modules. I'm not intimately familiar with this ecosystem (and not affiliated with webpacker gem at all), this npm library.

@babel/preset-env is a smart preset that allows you to use the latest JavaScript without needing to micromanage which syntax transforms (and optionally, browser polyfills) are needed by your target environment (s).

Looking at the docs:

Babel plugins - both full and shorthand names are supported, for example the following are functionally equivalent:

It looks like a feature of this library is to export plugins. If that's the case than a change in the name of available exported plugins would be a breaking change (again, this is a very weak assertion I'm not really that familiar with the ecosystem and want to lay my cards on the table).

The point of the preset is not to re-export all the plugins, but to abstract the plugins away from the Babel users. Instead of enabling the various plugins in the Babel config, that thus needs to be installed individually, the preset allows transforming all the necessary syntax without having to enable the individual plugins in the config. If a plugin is manually listed, it's "opting out" from being just enabled by default and thus its usage should not be considered as being enabled inside the preset, because it's not.

Unfortunately, we cannot fix this in webpacker or Rails 6 as both of them are no longer under development and locked to a specific major version of babel

I'm not familiar at all with how Rails works, doesn't it give access to a package.json files that users can edit?

That said, I'm thinking about how to fix this in a way that lets us handle @babel/preset-env's dependencies as an implementation detail without breaking major unmaintained packages using Babel. Even just a good error message explaining a workaround is better than the status quo.

@nicolo-ribaudo
Copy link
Member

@schneems I'm implementing a workaround for this specifically, but in order to do it I need to know how to detect if I'm running in a rails 6 app. I have never used anything in the Ruby ecosystem, and I'm struggling to get even a basic app do install 馃槄

Could you create one, and just commit everything and push it to GitHub, linking specifically to where the Babel config is?

@schneems
Copy link
Author

schneems commented Jun 6, 2023

Thanks for all the consideration in your response!

Even just a good error message explaining a workaround is better than the status quo.

100%

Another thought I had regarding my suggested solution of releasing another minor and major combo (which seems a bit heavy handed). There's possibly a less involved fix: add the old library back as a dependency as a no-op and add in a deprecation when people use it "hey this is going away, use 'transform' variant instead...

I'm not familiar at all with how Rails works, doesn't it give access to a package.json files that users can edit?

When you rails new it essentially acts as a site generator. It includes a bunch of templates and allows libraries to hook into that lifecycle as well. Rails 6 includes webpacker by default which is a wrapper around npm's webpack. It does include a package.json file that users can edit by default it looks like this:

{
  "name": "myapp",
  "private": true,
  "dependencies": {
    "@rails/actioncable": "^6.0.0",
    "@rails/activestorage": "^6.0.0",
    "@rails/ujs": "^6.0.0",
    "@rails/webpacker": "5.4.4",
    "turbolinks": "^5.2.0",
    "webpack": "^4.46.0",
    "webpack-cli": "^3.3.12"
  },
  "version": "0.1.0",
  "devDependencies": {
    "webpack-dev-server": "^3"
  }
}

Building assets will generate a yarn.lock file but it doesn't include one by default.

I need to know how to detect if I'm running in a rails 6 app.

I've got a tutorial https://devcenter.heroku.com/articles/getting-started-with-rails6 this is generated from a script that runs on my computer so what you see in the docs is what outputs from a real app https://github.com/zombocom/rundoc/blob/main/test/fixtures/rails_6/rundoc.md. Just stop before you get to the creating an app or deploying and you should have a working Rails app. Here's the end result of my script https://github.com/schneems/rails-6-simple-2023-06-06 (which includes an output of the tutorial docs it generates).

You'll need to install Ruby first. I recommend a ruby version manager like chruby and ruby-install (the two work together).

To determine your Rails version on the command line you can:

$ rails -v

If you have multiple versions of rails you can run <name> _<version>_ <args> like rails _6.1.7.3_ new myapp

It should generate some assets for you automatically. You can invoke production asset generation which invokes webpack via:

$ RAILS_ENV=production SECRET_KEY_BASE=asdf bundle exec rake assets:precompile assets:clean --trace

This should run a yarn install then a webpack compile. The --trace flag above shows you which tasks are running.

As a heads up I'm out for a bit on my 10 year anniversary.

@craftonixinc
Copy link

craftonixinc commented Mar 6, 2024

Is there any quick workaround for a newbie who doesn't want to get into the details?
I'm trying to follow the same path as @schneems and the Heroku workaround https://devcenter.heroku.com/articles/getting-started-with-rails6#fix-a-babel-regression still gives an error:

Compilation failed:
Hash: 50c91179df559b106792
Version: webpack 4.47.0
Time: 208ms
Built at: 2024-03-06 11:41:34 a.m.
 2 assets
Entrypoint application = js/application-a0a05a9c25d4f054cd70.js js/application-a0a05a9c25d4f054cd70.js.map
[0] ./app/javascript/packs/application.js 1.61 KiB {0} [built] [failed] [1 error]

ERROR in ./app/javascript/packs/application.js
Module build failed (from ./node_modules/babel-loader/lib/index.js):
Error: [BABEL]: --- PLACEHOLDER PACKAGE ---
This @babel/plugin-proposal-private-property-in-object version is not meant to
be imported. Something is importing
@babel/plugin-proposal-private-property-in-object without declaring it in its
dependencies (or devDependencies) in the package.json file.
Add "@babel/plugin-proposal-private-property-in-object" to your devDependencies
to work around this error. This will make this message go away.

I followed what the error message says, so I added "@babel/plugin-proposal-private-property-in-object" to my devDependencies as follows:

diff --git a/babel.config.js b/babel.config.js
index 19a07f3..f42d298 100644
--- a/babel.config.js
+++ b/babel.config.js
@@ -54,7 +54,7 @@ module.exports = function(api) {
         }
       ],
       [
-        '@babel/plugin-proposal-private-methods',
+        '@babel/plugin-transform-private-methods',
         {
           loose: true
         }
diff --git a/package.json b/package.json
index b3f2a56..8d6560b 100644
--- a/package.json
+++ b/package.json
@@ -12,6 +12,7 @@
   },
   "version": "0.1.0",
   "devDependencies": {
-    "webpack-dev-server": "^3"
+    "webpack-dev-server": "^3",
+    "@babel/plugin-proposal-private-property-in-object": "^7.21.11"
   }
 }

but I'm still getting the same error.

@nicolo-ribaudo
Copy link
Member

Can you check in your project where @babel/plugin-proposal-private-property-in-object is used, and use @babel/plugin-transform-private-property-in-object instead? (adding it to your deps)?

@marcioecom
Copy link

@craftonixinc I installed @babel/plugin-proposal-private-methods and @babel/plugin-proposal-private-property-in-object, it worked for me

yarn add @babel/plugin-proposal-private-methods @babel/plugin-proposal-private-property-in-object

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants