Skip to content

Commit

Permalink
fix: keep the initial slash for fetching VERSION.txt (#99)
Browse files Browse the repository at this point in the history
* chore: prettier

* fix: update to glimmer component

BREAKING CHANGE: use GC for notifier

* fix: update waitFor

* fix: update deps

* feat: Extract new version fetching and checking from the NewVersionNotifier component in a service (#94)

BREAKING CHANGE: introduces a new version service. Some arguments from the components have been moved here and other moved to the configuration.
BREAKING CHANGE: How the version check happens has also changed, see #94

* Enable JS code checking

* Extract new version fetching and checking from the NewVersionNotifier component in a service

The logic is now in a service so that it can be used independently of the action.

There are slight behaviour changes and new behaviours as well.

The service contains an observable `isNewVersionAvailable` property.
This property is set to true when a new version is available.

The version checking is slightly different: before, the first check was ignored because the component was comparing the previously fetched version and the latest fetched version.
Now, the version is always checked against the version provided in environment.js. This thus means the comparison is always done against the compiled version of the app.

The component had a way to ignore a version, by setting the next version to check to the latest fetched version. To keep the compatibility with this behaviour, there is now an explicit list of "ignore versions" in the service, populated when the user closes the `NewVersionNotifier`. Note that this list is kept in memory only and when the user reloads the page, the list is empty again. This is the same behaviour as before.

I kept the code related to testing in the service to limit the changes, but this could be moved in the TestNewVersionService implementation so that the original NewVersionService is kept clean of testing behaviour.

The onNewVersion action still exists but is now in the service. If one needs this callback, he/she can overload the NewVersionService and override onNewVersion. Same thing for onError.

Some properties that were arguments of the component have been moved to the configuration.

* Fix dependencies declaration

Some addon dependencies were declared in devDependencies while they are needed by the addon to run.

* Make MAX_COUNT_IN_TESTING a configuration variable

This let tests override this option as needed.

* fix: keep the initial slash for fetching VERSION.txt

Before this commit, the fetch of VERSION.txt with a default configuration would result to calling "VERSION.txt", which means "fetch the file relative to the current page URL".

This may lead to issue in production, but leads to issues in some tests situation when using `ember test` where the full path to VERSION.txt is resolved to `/VERSION.txt` when declaring it in tests, while it is resolved to `http://localhost:7357/some-random-numbers/tests/VERSION.txt` when fetching the file.

This commit fixes the issue by not removing the `/` in front of VERSION.txt.

This fixes the isse for my scenarios; but I am not sure why there was this check and removal of the `/` in the first place, this thus may lead to breaking changes for some.

Co-authored-by: Ilya Radchenko <knownasilya@gmail.com>
  • Loading branch information
pbernery and knownasilya committed Nov 24, 2021
1 parent f3dda01 commit 1daf826
Showing 1 changed file with 0 additions and 5 deletions.
5 changes: 0 additions & 5 deletions addon/services/new-version.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,6 @@ export default class NewVersionService extends Service {
const versionFileName = this._newVersionConfig.versionFileName;
const baseUrl =
this._config.prepend || this._config.rootURL || this._config.baseURL;

if (!this._config || baseUrl === '/') {
return versionFileName;
}

return baseUrl + versionFileName;
}

Expand Down

0 comments on commit 1daf826

Please sign in to comment.