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

Update MoJ frontend, dropping jQuery which is no longer required #303

Merged
merged 3 commits into from
Jan 26, 2024

Conversation

ushkarev
Copy link
Contributor

@ushkarev ushkarev commented Jan 26, 2024

Might be worth noting that cypress still bundles jquery so it will remain available for integration tests

).forEach(dir => {
router.use('/assets', express.static(path.join(process.cwd(), dir), cacheControl))
})

Array.of('/node_modules/govuk_frontend_toolkit/images').forEach(dir => {
router.use('/assets/images/icons', express.static(path.join(process.cwd(), dir), cacheControl))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you mean to remove this too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup i explained in that commit that we don't even install the deprecated govuk_frontend_toolkit package

Copy link
Contributor

@petergphillips petergphillips left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ushkarev ushkarev merged commit 2afb131 into main Jan 26, 2024
4 checks passed
@ushkarev ushkarev deleted the drop-jquery branch January 26, 2024 10:49
@yndajas
Copy link
Contributor

yndajas commented Jan 30, 2024

Seems the sortable table component still needs jQuery by the way

@ushkarev
Copy link
Contributor Author

ushkarev commented Jan 30, 2024

@yndajas hmm that's weird… they announced that the 2.1 update

also removes a dependency on using jQuery within MOJ frontend code

… so i guessed they missed a component? i presume their intention is to actually remove the jquery dependency so it might be worth raising it with them?

@yndajas
Copy link
Contributor

yndajas commented Jan 30, 2024

… so i guessed they missed a component? i presume their intention is to actually remove the jquery dependency so it might be worth raising it with them?

Odd! There's a still-open issue from 2021 about this: ministryofjustice/moj-frontend#134. jQuery is definitely still being used on main: https://github.com/search?q=repo:ministryofjustice/moj-frontend+$.&type=code. Will ask about the status of the dependency on Slack

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.

3 participants