-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
[Bug]: Breaking change in a minor version bump #15679
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
Comments
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. |
What's happening here is that your config is depending on The Rust RFC you mentioned doesn't include "renaming a crate" as a breaking change, because it's not:
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
Now that The workaround you found in rails/rails#48372 (comment) is a bad workaround: it still relies on a package been present in your Two good solutions are:
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)
}
}
Additionally, I believe that |
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:
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.
Looking at the docs:
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 But I agree that the current consequences are a bit annoying, maybe we can consider keeping the old dependencies for now. |
There are three package managers in the JavaScript ecosystem: npm, pnpm and yarn. By default, both pnpm and yarn prevent this problem:
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.
I'm not familiar at all with how Rails works, doesn't it give access to a That said, I'm thinking about how to fix this in a way that lets us handle |
@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? |
Thanks for all the consideration in your response!
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...
When you
Building assets will generate a
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 To determine your Rails version on the command line you can:
If you have multiple versions of rails you can run It should generate some assets for you automatically. You can invoke production asset generation which invokes webpack via:
This should run a yarn install then a webpack compile. The As a heads up I'm out for a bit on my 10 year anniversary. |
Is there any quick workaround for a newbie who doesn't want to get into the details? 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. |
Can you check in your project where |
@craftonixinc I installed yarn add @babel/plugin-proposal-private-methods @babel/plugin-proposal-private-property-in-object |
💻
How are you using Babel?
babel-loader (webpack)
Input code
Configuration file name
babel.config.js
Configuration
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:
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 inbabel.config.js
is no longer installed and this causes an error.Environment
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 💜
The text was updated successfully, but these errors were encountered: