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

Add support to Shakapacker #158

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tagliala
Copy link

@tagliala tagliala commented May 19, 2024

Shakapacker is the official, actively maintained successor to Webpacker.

Shakapacker v7 changed the spelling of Webpacker to Shakapacker in
the entire project, but still provided backward compatibility for
Webpacker spelling.

v8 dropped the deprecated spelling

This commit also:

  • Checks if Shakapacker is defined; if not, it falls back on
    Webpacker.
  • Uses the scope resolution operator to resolve at top-level
    scope
  • Checks protocol instead of https? because the former is available
    from Webpacker 3 and the latter is not available anymore in
    Shakapacker >= 8

Refs:

Close #156

@tagliala tagliala force-pushed the feat/support-shakapacker branch 2 times, most recently from 770434c to 9c0ecc4 Compare May 24, 2024 06:48
@tagliala tagliala marked this pull request as ready for review May 25, 2024 06:30
@tagliala tagliala requested a review from paypro-leon May 25, 2024 06:30
Shakapacker is the official, actively maintained successor to Webpacker.

Shakapacker v7 changed the spelling of `Webpacker` to `Shakapacker` in
the entire project, but still provided backward compatibility for
`Webpacker` spelling.

v8 dropped the deprecated spelling

This commit also:
- Checks if `Shakapacker` is defined; if not, it falls back on
  `Webpacker`.
- Uses the scope resolution operator to resolve at top-level
  scope
- Checks `protocol` instead of `https?` because the former is available
  from Webpacker 3 and the latter is not available anymore in
  Shakapacker >= 8

Refs:
- shakacode/shakapacker#414
- shakacode/shakapacker#429
- shakacode/shakapacker#486

Close jamesmartin#156
@paypro-leon
Copy link

@tagliala Looks good to me. Just need a maintainer to have a look and review.

@tagliala
Copy link
Author

Thanks.

I have two successful builds on Shakapacker 7 and 8 at:

A proper solution would be to create a new app with Shakapacker 8 and make an integration test here, as per #159, but this would require to fix the test apps repo

@tagliala
Copy link
Author

I have a working build against Rails 7.1 / Shakapacker 8: https://github.com/tagliala/inline_svg/actions/runs/9549035858

I've removed the old test against webpacker because of python 2.7 and deprecations.

Please let me know if you need some help or if I should fork

@jamesmartin
Copy link
Owner

Please let me know if you need some help or if I should fork

Thanks for your work so far. Sorry for the delayed responses, I was away on vacation and then work trips. Let's get CI working properly and then get this merged.

@tagliala
Copy link
Author

Hi,

Let's get CI working properly and then get this merged.

I did not invest time in webpacker (it should have been upgraded from v4 to v5 in the rails 6 branch), so this is expected to fail against the CI like it is failing now

#159 is based on this PR and should make the CI pass agains Rails 7.1 / Shakapacker 8

If you are interested, I can make a PR to drop compatibility with legacy Rails and Ruby versions

@jamesmartin
Copy link
Owner

If you are interested, I can make a PR to drop compatibility with legacy Rails and Ruby versions

Sounds good. Let's do that. 👍

@tagliala
Copy link
Author

tagliala commented Jul 20, 2024

Friendly bump.

The first steps would be to merge:

Then this PR can be merged because it is not breaking (and it is tested in #159 against Shakapacker)

Then, after a rebase and some changes to the target repo in actions, it will be possible to merge:

To have a working CI.

At that point, it would be possible to release a minor version with proper Shakapacker 7+ support.

After the minor release, it would be possible to consider to drop old Ruby and Rails support and go for 2.0. I would entirely drop webpacker support for 2.0

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.

Support Shakapacker 8
3 participants