Extract new version fetching and checking from the NewVersionNotifier component in a service #94
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… 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.
Some addon dependencies were declared in devDependencies while they are needed by the addon to run.
knownasilya
pushed a commit
that referenced
this pull request
Jun 28, 2021
…tifier 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.
knownasilya
added a commit
that referenced
this pull request
Jun 29, 2021
* 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>
knownasilya
pushed a commit
that referenced
this pull request
Nov 24, 2021
…tifier 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.
knownasilya
added a commit
that referenced
this pull request
Nov 24, 2021
* 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>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
isNewVersionAvailableproperty. 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.