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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed webpack 4 builds breaking due to nullish coalescing operator #3679

Merged
merged 2 commits into from Sep 11, 2023

Conversation

mgschoen
Copy link
Contributor

What's the issue?

After upgrading to the latest version of alpinejs in a webpack 4 project, I discovered that my build was broken due to webpack encountering the unexpected token ?? in Alpine's source. This is the nullish coalescing operator and it was introduced in alpinejs@3.12.1 (#3483). It's fairly common nowadays. It wasn't though when webpack 4 was a thing, and it turned out webpack 4 never got support for it. See this nice write-up of the same problem in a completely unrelated project: PaulLeCam/react-leaflet#883

Proposal

The problem could be solved on the consumer side by adding transpilation for ??. I'd still suggest fixing it on the library side for the following reasons:

  • this kind of deprecation should probably only occur in a major version
  • the webpack 4 -> webpack 5 migration is pretty painful especially in complex setups and I suppose my project is not the only one that didn't yet make the jump
  • ?? can easily be replaced and we get extended webpack 4 support with virtually no effort

What do you guys think?

@ekwoka
Copy link
Contributor

ekwoka commented Jul 27, 2023

This seems improper.

Nullish coalescing is basically from the same time that Alpines minimum features are from.

If you haven't updated Webpack in 3 years, then why would you need to update Alpine now?

The CDN version is already transpiled to target ES 2017 if you need that.

@mgschoen
Copy link
Contributor Author

Thanks for your thoughts. I agree we're quite late with the webpack update. But I'd say outdated build tools are not a super uncommon thing, aren't they? We're trying to keep at least the code we ship to users up to date, so updating Alpine is definitely a priority.

@SimoTod
Copy link
Collaborator

SimoTod commented Jul 28, 2023

Don't you just need to use https://babeljs.io/docs/babel-plugin-transform-nullish-coalescing-operator in your build process?

@mgschoen
Copy link
Contributor Author

@SimoTod Sure, that works. But my proposal is fixing it on the library side so people don't need a transpiler to be able to use the library. Feel free to reject if this is not a priority for you, we'll be fine without the change!

@SimoTod
Copy link
Collaborator

SimoTod commented Jul 28, 2023

Sure, that works. But my proposal is fixing it on the library side so people don't need a transpiler to be able to use the library. Feel free to reject if this is not a priority for you, we'll be fine without the change!

No worries, I wasn't sure you were aware of the option. I don't have decisional power, the maintainer will review and decide.

@UnleashedMySelf
Copy link

Hi everyone, maybe we can still persuade the developers to replace 'nullish coalescing operator' with es5 format? :) it's just the only issue that makes AlpineJS not work on ios 12-13.. a simple format change will cover these two versions, this is very important
@calebporzio pleeeasee

@ekwoka
Copy link
Contributor

ekwoka commented Aug 2, 2023

@UnleashedMySelf

it's just the only issue that makes AlpineJS not work on ios 12-13

The CDN already transpiles this, and you can set your own bundling target (and should be).

nullish coalescing is transpilable, so it's better to set up your tooling for the goals you have than to hold the rest of the crabs in the bucket.

@calebporzio
Copy link
Collaborator

Because there are so few instances of "??" I'm going to merge this, but I agree we should be able to use these syntaxes and may in the future. Thanks. everyone

@calebporzio calebporzio merged commit ba08ec4 into alpinejs:main Sep 11, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants