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

Allow using nested entries #121

Merged
merged 7 commits into from
Jun 3, 2022
Merged

Allow using nested entries #121

merged 7 commits into from
Jun 3, 2022

Conversation

justin808
Copy link
Member

Nested entries were removed in v6.0.

This PR allows using them with a configuration switch.

By default they are turned off.

@@ -8,6 +8,9 @@ Changes since last non-beta release.

*Please add entries here for your pull requests that are not yet released.*

## Added
- Configuration boolean option `nested_entries` to use nested entries. This was the default prior to v6.0. Because entries maybe generated, it's useful to allow a `generated` subdirectory.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@justin808 can we add something in the readme suggesting when you would want to use that and examples?

@@ -34,7 +37,7 @@ Note: [Rubygem is 6.3.0.pre.rc.1](https://rubygems.org/gems/shakapacker/versions
### Improved
- Use last modified timestamps rather than file digest to determine compiler freshness. [PR 97](https://github.com/shakacode/shakapacker/pull/97) by [tomdracz](https://github.com/tomdracz).

Rather than calculating SHA digest of all the files in the paths watched by the compiler, we are now comparing the modified time of the `manifest.json` file versues the latest modified timestamp of files and directories in watched paths. Unlike calculating digest, which only looked at the files, the new calculation also considers directory timestamps, including the parent ones (i.e. `config.source_path` folder timestamp will be checked together will timestamps of all files and directories inside of it).
Rather than calculating SHA digest of all the files in the paths watched by the compiler, we are now comparing the modified time of the `manifest.json` file versus the latest modified timestamp of files and directories in watched paths. Unlike calculating digest, which only looked at the files, the new calculation also considers directory timestamps, including the parent ones (i.e. `config.source_path` folder timestamp will be checked together will timestamps of all files and directories inside of it).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good catch!

})

test('should return 3 entry points with config.nested_entries == true', () => {
expect(baseConfig.entry.multi_entry.length).toEqual(2)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Test description is saying it will return 3 entrypoints but test expectation is matching length of 2 🤔 Something is not quite right here

resolve('app', 'packs', 'entrypoints', 'multi_entry.js')
])
expect(baseConfig.entry['generated/something']).toEqual(
resolve('app', 'packs', 'entrypoints', 'generated', 'something.js')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like this is failing ATM

justin808 added 2 commits June 1, 2022 16:19
Nested entries were removed in v6.0.

This PR allows using them with a configuration switch.

By default they are turned off.
@justin808
Copy link
Member Author

@tomdracz did I get all your feedback?

@justin808 justin808 requested a review from tomdracz June 2, 2022 02:30
Copy link
Collaborator

@tomdracz tomdracz left a comment

Choose a reason for hiding this comment

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

Good to go after the spec failure is sorted

@justin808 justin808 merged commit 6a977bf into master Jun 3, 2022
@justin808 justin808 deleted the nested-entries branch June 3, 2022 01:48
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