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

Define jQuery object in separate file to support esbuild #3571

Merged
merged 6 commits into from
Dec 3, 2022

Conversation

wayc
Copy link
Contributor

@wayc wayc commented Nov 22, 2022

Background

During our migration from webpacker to esbuild, our apps using rails_admin began to fail during builds, along with Chrome showing a jQuery is not defined error in the Console:

image

We traced the root cause to the order of these actions in src/rails_admin/base.js:

The jquery-ui modules are imported before window.$ is defined, causing the above issues.

To add support for esbuild and to avoid issues with async and hoisting, we suggest importing and initialising jQuery together.

Changes

  • Move the jQuery initialiser into a separate file to import and define jQuery together

@coveralls
Copy link

coveralls commented Nov 22, 2022

Coverage Status

Changes unknown
when pulling c1235a5 on wayc:master
into ** on railsadminteam:master**.

@wayc wayc changed the title Import jQuery in separate file to avoid hoisted variables Define jQuery object in separate file to allow integration with esbuild Nov 22, 2022
@wayc wayc changed the title Define jQuery object in separate file to allow integration with esbuild Define jQuery object in separate file to support esbuild Nov 22, 2022
@rjaus
Copy link

rjaus commented Nov 23, 2022

Nice work. Are these the only changes you made to get rails_admin running with esbuild? I'm struggling to get it to work.

@wayc wayc force-pushed the master branch 2 times, most recently from 61d2b15 to 32a5f84 Compare November 24, 2022 21:43
@wayc
Copy link
Contributor Author

wayc commented Nov 24, 2022

Nice work. Are these the only changes you made to get rails_admin running with esbuild? I'm struggling to get it to work.

@rjaus Hi there 👋

Yes, the changes in this PR allowed esbuild to build successfully. However, our migration from webpacker to esbuild took some effort. If you're in a similar situation, you can try creating a new separate Rails project on esbuild: rails new demo-rails -j esbuild and implementing rails_admin. Then you can diff between your new and existing projects to identify areas to change.

It looks like a few test suites are failing here—I'll sort them out soon.

@wayc wayc mentioned this pull request Nov 25, 2022
@wayc wayc marked this pull request as ready for review November 26, 2022 15:46
@wayc wayc marked this pull request as draft November 26, 2022 16:09
@wayc wayc marked this pull request as ready for review November 26, 2022 16:12
@wayc
Copy link
Contributor Author

wayc commented Nov 26, 2022

@mshibuya @pcai

Thank you so much for resolving the failing JRuby tests in #3574

I've marked this PR (on jQuery initialisation) as ready to review and would love to hear your thoughts.

@Zeneixe
Copy link

Zeneixe commented Dec 1, 2022

Hello, any updates on this?

@mshibuya mshibuya merged commit 4a864aa into railsadminteam:master Dec 3, 2022
@mshibuya
Copy link
Member

mshibuya commented Dec 3, 2022

Confirmed to work, thanks!

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.

5 participants