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

If WebJar version is a "-snapshot", overwrite existing files when extracting #159

Merged
merged 1 commit into from
Jun 8, 2024

Conversation

mkurz
Copy link

@mkurz mkurz commented Jun 7, 2024

In general, Snapshots usually indicate things are still wip and changing (e.g., that's why you can overwrite artifacts in the sonatype snapshot repository); therefore, existing files should get overwritten when extracting a webjar which obviously is a -snapshot release.

This change alone makes total sense on its own and will not break things, IMHO.

However, the real reason we need this behavior is because it helps us (Play) to fix:

While developing a Play application and editing assets (css, js, etc.) in a sub-project, the sub-project's version usually is a -snapshot, so when a user edits an asset and refreshes the browser, the updated asset (which is internally a webjar) should be served. Currently, this is broken because files will not get overwritten. (I did test this locally with Play apps, and it works with this PR.)

@jamesward If you give thumbs up for this PR, would be nice if you could cut a new release also, thanks!

@jamesward
Copy link
Member

This seems good to me. The only issue is that for some reason the behavior is different on Windows. :(

@mkurz mkurz force-pushed the allow_resource_overwrite branch from e5f9aeb to f3e6258 Compare June 8, 2024 00:02
@mkurz
Copy link
Author

mkurz commented Jun 8, 2024

The only issue is that for some reason the behavior is different on Windows. :(

Fixed, this was just a new line quirk in the test I added.

BTW. In Play 2.8 this was working because it was using a very outdated webjars-locator-core version that did not have

merged yet.
With this PR you switched from "do not overwrite a file when it is up to date" to "do not overwrite a file when it exists". You can see that also in the tests you changed: extractWebJarShouldNotExtractWhenWhenUpToDate got removed and extractWebJarShouldNotExtractWhenFileExists got added. (diff).
So, in Play 2.8 when a user edited a file, it was always newer than the previous extracted one and so it was never "up to date" and the file got overwritten.

Thanks!

@jamesward jamesward merged commit f0ecec4 into webjars:main Jun 8, 2024
2 checks passed
@mkurz mkurz deleted the allow_resource_overwrite branch June 8, 2024 00:09
@jamesward
Copy link
Member

This is being released as 0.59

@mkurz
Copy link
Author

mkurz commented Jun 8, 2024

You rock!

@jamesward
Copy link
Member

No, Matthias. You totally rock! :)

mkurz added a commit to mkurz/playframework that referenced this pull request Jun 10, 2024
mkurz added a commit to mkurz/playframework that referenced this pull request Jun 10, 2024
mkurz added a commit to mkurz/playframework that referenced this pull request Jun 10, 2024
mergify bot pushed a commit to playframework/playframework that referenced this pull request Jun 10, 2024
mergify bot pushed a commit to playframework/playframework that referenced this pull request Jun 10, 2024
Contains webjars/webjars-locator-core#159
to fix #12484

(cherry picked from commit 6b295a3)

# Conflicts:
#	project/Dependencies.scala
mkurz added a commit to playframework/playframework that referenced this pull request Jun 10, 2024
Contains webjars/webjars-locator-core#159
to fix #12484

(cherry picked from commit 6b295a3)

# Conflicts:
#	project/Dependencies.scala
mkurz added a commit to playframework/playframework that referenced this pull request Jun 10, 2024
Contains webjars/webjars-locator-core#159
to fix #12484

(cherry picked from commit 6b295a3)

# Conflicts:
#	project/Dependencies.scala
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