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

Download and bundle third party JS packages with npm #2679

Merged
merged 17 commits into from
Jan 9, 2024

Conversation

tvdeyen
Copy link
Member

@tvdeyen tvdeyen commented Jan 8, 2024

What is this pull request for?

Do not use CDNs any more to load ShoelaceJS or use downloaded and vendored third party scripts.

This fixes issues with

  • Speed
  • GDPR
  • Outdated versions
  • Reliability

etcpp.

Notable changes

We now use npm to download and manage JS dependencies and Rollup to build packages that we then import
via importmaps.

Checklist

importmap-rails allows to download pinned assets
instead of serving them via CDN.

We want to do that for several reasons:

- Reliability
- GDPR
- Speed
This function is so small that using lodash is not worth it.
And remove lodash-max
We do not use it anymore
importmap-rails v2 will have this as default as well.
It is not possible to pin and download shoelace
with importmap-rails. Since it is more efficient to have
a bundle anyway and depency management via yarn
much easier than with importmap-rails we install it
via yarn and build it with rollup. This can easyliy be
automated with github actions, so we have best of
all worlds.
We already build our own optimized shoelace package.
Since we already use yarn to handle dependencies we
download all of our third party dependencies and build
a packages for them and make them available via source
map.

That way we can make use of automated dependency updates
(like dependabot) and automatically build the packages with
github actions. That would be significantly harder with importmap-
rails.
This makes the bundled packages significantly smaller.
This way we can use npm as dependency manager
and omit loading each plugin on its own
Instead of a downloaded version from vendor/assets folder.
So we can use established tools for dependency updates.
This way our JS dependencies get version updates.
After new versions installed build new packages.
All browsers support it now.
@tvdeyen tvdeyen added this to the 7.1 milestone Jan 8, 2024
@tvdeyen tvdeyen requested a review from a team January 8, 2024 17:55
@tvdeyen tvdeyen changed the title Download third party JS packages Download and bundle third party JS packages Jan 8, 2024
Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 5328 lines exceeds the maximum allowed for the inline comments feature.

We do not need these files in the package Rubygem.
The yarn.lock is very large and all the dot files are only
necessary for local development.
Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 5330 lines exceeds the maximum allowed for the inline comments feature.

Now that we have modules in `vendor/javascript` and in
`vendor/assets/javascripts` we need to change the module
mapping for jest specs.
Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 5341 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 5657 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 5675 lines exceeds the maximum allowed for the inline comments feature.

We maybe want to add JS dependencies in PRs not
only through dependabot PRs.
Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 5679 lines exceeds the maximum allowed for the inline comments feature.

Copy link
Contributor

@sascha-karnatz sascha-karnatz left a comment

Choose a reason for hiding this comment

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

Well done!

@tvdeyen tvdeyen changed the title Download and bundle third party JS packages Download and bundle third party JS packages with npm Jan 9, 2024
@tvdeyen tvdeyen merged commit 9249d01 into AlchemyCMS:main Jan 9, 2024
33 checks passed
@tvdeyen tvdeyen deleted the download-imported-pins branch January 9, 2024 10:27
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.

2 participants