Skip to content

Commit

Permalink
Strip additional_paths from the asset paths generated in the file.js …
Browse files Browse the repository at this point in the history
…rule (#403)

* Add config.includePaths and strip includePaths from file.js output paths

* Update CHANGELOG.md

* Refactor code to fix lint issue

* Make sure to replace only once

* Change variable name

* Update README.md

* Grammer change

* Revert includePath change and make stripping additional_paths opt-in

* Update CHANGELOG.md

* Revert some more changes

* Revert changes

* Remove opt-in check

* Update README and CHANGELOG
  • Loading branch information
paypro-leon authored and ahangarha committed Mar 28, 2024
1 parent 773b5ba commit e33dce9
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 3 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ Changes since the last non-beta release.

- Enable `ensure_consistent_versioning` by default [PR 447](https://github.com/shakacode/shakapacker/pull/447) by [G-Rath](https://github.com/g-rath).

- Asset files put in `additional_paths` will have their path stripped just like with the `source_path`. [PR 403](https://github.com/shakacode/shakapacker/pull/403) by [paypro-leon](https://github.com/paypro-leon).

## [v7.2.3] - March 23, 2024

### Added
Expand Down
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -899,6 +899,12 @@ import 'stylesheets/main'
import 'images/rails.png'
```

Assets put in these folders will also have their path stripped just like with the `source_path`.

Example:

A file in `app/assets/images/image.svg` with `additional_paths: ['app/assets']` will result in `static/images/image.svg`

**Note:** Please be careful when adding paths here otherwise it will make the compilation slow, consider adding specific paths instead of the whole parent directory if you just need to reference one or two modules

**Also note:** While importing assets living outside your `source_path` defined in shakapacker.yml (like, for instance, assets under `app/assets`) from within your packs using _relative_ paths like `import '../../assets/javascripts/file.js'` will work in development, Shakapacker won't recompile the bundle in production unless a file that lives in one of it's watched paths has changed (check out `Shakapacker::MtimeStrategy#latest_modified_timestamp` or `Shakapacker::DigestStrategy#watched_files_digest` depending on strategy configured by `compiler_strategy` option in `shakapacker.yml`). That's why you'd need to add `app/assets` to the additional_paths as stated above and use `import 'javascripts/file.js'` instead.
Expand Down
18 changes: 18 additions & 0 deletions package/rules/__tests__/file.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
const file = require('../file')

jest.mock("../../config", () => {
const original = jest.requireActual("../../config");
return {
...original,
additional_paths: [...original.additional_paths, "app/assets"],
};
});

describe('file', () => {
test('test expected file types', () => {
const types = [
Expand Down Expand Up @@ -59,4 +67,14 @@ describe('file', () => {
'static/images/nested/deeply/[name]-[hash][ext][query]'
);
});

test('correct generated output path is returned for additional_paths', () => {
const pathData = {
filename: 'app/assets/images/image.svg',
};

expect(file.generator.filename(pathData)).toEqual(
'static/images/[name]-[hash][ext][query]'
);
});
})
14 changes: 11 additions & 3 deletions package/rules/file.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,22 @@
const { dirname } = require('path')
const { source_path: sourcePath } = require('../config')
const {
additional_paths: additionalPaths,
source_path: sourcePath
} = require('../config')

module.exports = {
test: /\.(bmp|gif|jpe?g|png|tiff|ico|avif|webp|eot|otf|ttf|woff|woff2|svg)$/,
exclude: /\.(js|mjs|jsx|ts|tsx)$/,
type: 'asset/resource',
generator: {
filename: (pathData) => {
const folders = dirname(pathData.filename)
.replace(`${sourcePath}`, '')
const path = dirname(pathData.filename)
const stripPaths = [...additionalPaths, sourcePath]

const selectedStripPath = stripPaths.find((includePath) => path.includes(includePath))

const folders = path
.replace(`${selectedStripPath}`, '')
.split('/')
.filter(Boolean)

Expand Down

0 comments on commit e33dce9

Please sign in to comment.