The team had feedback during a sprint retro that the breaking change
process documentation was a bit spread out. In particular, some team
members were not clear that you need to a) update the breaking.md file
and b) add the "BREAKING CHANGE" comment to the commit description.
This PR adds both of these things are comments for awareness.
I've noticed a few instances where the team forgets to add the
"resolves" keyword (or similar "closes"/"fixes" keywords) when linking a
PR to an issue. GitHub will not link the PR with the issue and will not
close the issue on PR merge unless one of these keywords is used.
I've updated the PR template to add the "resolves" keyword so team
members don't need to worry about remembering to add it.
Issue number: N/A
---------
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
The labeler action is currently failing:
https://github.com/ionic-team/ionic-framework/actions/runs/7090913880/job/19298918578?pr=28622.
This is happening due to a breaking change in v5 of the action.
We currently pull from `main` for this action so we are now receiving v5
of the action:
fe3c3d500a/.github/workflows/label.yml (L16)
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- This PR updates the action explicitly to v5 so we don't unexpectedly
take on breaking changes
- This PR also updates the labeler yaml file to account for the v5
breaking changes
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Issue number: Internal
---------
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
There is no Angular v17 test app.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
There is an Angular v17 test app.
New overrides:
- angular.json: `browserTarget` was changed to `buildTarget` and to
replace `@nguniversal` with `@angular/ssr` and
`@angular-devkit/build-angular`
- server.ts: Replace uses of `@nguniversal` with `@angular/ssr`
New change to base:
- polyfills.ts: Import
[changed](https://github.com/angular/angular/blob/main/CHANGELOG.md#zonejs)
from `import 'zone.js/dist/zone';` (this change is supported for all
versions of Angular that we support, so should be changed in the base
file)
All other files were duplicated from their v16 counterparts.
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Issue number: #
---------
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
i found a small typo in the contributing guide
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
update the relative path for updating ionic angular, now that it's code
lives under the `packages/` directory
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Issue number: N/A
---------
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
Testing Ionicons in Ionic is currently difficult. Developers need to a)
create a dev build of Ionicons, b) create a branch in Ionic, c) install
the dev build in Ionic core, d) push the branch, and e) create a draft
PR to watch the CI process run.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- Introduces a workflow dispatch option for our CI process. This allows
developers to run the CI process for any branch without creating PR.
- I also added an optional input so devs can specify the ionicons
version if they want to pass a special version.
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Issue number: N/A
"Owning" a test didn't really work out like I expected/hoped. A lot of
changes to the test files were minor, so I ended up just getting
assigned to more-or-less random PRs. It wasn't really hurting anything,
but I might as well just let the bot assign me.
---------
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
Issue number: N/A
---------
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
when the stencil team wants to test a dev build of stencil against the
framework team's ci, they push a branch to this repo that overrides the
stencil nightly job's npm tag (replacing 'nightly' with the dev build
version) and manually kick off the nightly workflow. this commit would
eliminate the need for that first step there
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
update the stencil nightly job to accept any npm release tag, rather
than always default to 'nightly'. doing so allows the stencil team to
reuse this workflow for cases where we'd like to test a _dev_ build of
stencil by running it through the framework's ci process, without
landing the feature in stencil first.
I was able to test that `nightly` gets set by default (and the field is
required) by running the workflow from this branch.
I then also tested this against a dev build of Stencil:
Interestingly enough, it helped me catch something to consider if we
were to accept the PR this dev build is accepted!
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
I considered making a separate workflow for this (rather than
override/use nightly) - it didn't quite seem worth the maintenance
effort 🤷
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Issue number: N/A
---------
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
We checked `git diff` to see if new screenshots were generated. If no
new screenshots were generated then we print a user-friendly message
informing the dev what happened. However, `git diff` does not account
for untracked changes (i.e. new screenshots).
As a result, this prevented new screenshots from being committed.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- To fix this we use the `-N` flag which is short for `--intent-to-add`.
This adds an entry with no content which causes the new screenshots to
show up when running `git diff`. We later add the contents of the image.
Example test run of this working:
https://github.com/ionic-team/ionic-framework/actions/runs/6174263712/job/16759260534
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Issue number: N/A
---------
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
When the "Update Reference Screenshot" job is run but no new screenshots
are generated, the following error is logged:
```
nothing added to commit but untracked files present (use "git add" to track)
Error: Process completed with exit code 1.
```
This is happening because there are no files to commit when running `git
commit`. Brandy noted that this was confusing since it doesn't actually
tell you why there are no files to commit.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- If there are no diffs to commit then the update screenshot script will
log a message saying that no screenshot diffs were generated.
Example:
```
⚠️ Error: No new screenshots generated ⚠️
This means that there were zero visual diffs when running screenshot tests.
Make sure you have pushed any code changes that would result in visual diffs.
```
https://github.com/ionic-team/ionic-framework/actions/runs/6099399582/job/16552017414
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Issue number: N/A
---------
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
`action/setup-node@v3` shipped a "breaking" change that requires node
v18. This breaks our dev build release process.
https://github.com/ionic-team/ionic-framework/actions/runs/6044495970/job/16403194005
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- Updates the publish action to use node v18
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Here is a successful run with these changes:
https://github.com/ionic-team/ionic-framework/actions/runs/6044521110
Issue number: N/A
---------
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
Maria isn't part of the codeowners.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- Maria has been added to the Angular and Vue folders.
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
N/A
Issue number: #
---------
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
-
-
-
## Does this introduce a breaking change?
- [ ] Yes
- [ ] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Issue number: N/A
---------
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
When the CI process detects a diff between the output in CI after
running `npm run build` versus the checked out branch, it will log the
diff and exit. It isn't very clear to developers what they need to do or
why this job step exists.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- Logs a prettier error message to the developer to let them know the
steps they can take when a diff is detected in CI
Example run:
https://github.com/ionic-team/ionic-framework/actions/runs/5942940488/job/16117074372?pr=28044
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Issue number: #
---------
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
Stencil v3.4 is being used in the nightly run of these tests
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
this commit updates the `test-core-spec` action to accept an argument
for the version of stencil that should be installed/used. since we do
not (and arguably, should not) cache `node_modules/`, we need a way to
inform this action that a nightly version of stencil should be installed
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
I fully expect nightly CI to begin to fail as a result of merging this.
We'll fix that next week (in Stencil)
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Issue number: Internal
---------
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
Ionic currently detects and uses Capacitor APIs for different plugins
(haptics, status bar and keyboard). This implementation does not have
type safety and can result in unexpected behaviors.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- Adds `@capacitor/core`, `@capacitor/keyboard`, `@capacitor/haptics`
and `@capacitor/status-bar` as dev dependencies. These should _only_ be
used with `import type { }`.
- Refactors the plugin usages to be typed against the plugin packages,
while using a duplicate enum when needing a value. This allows us to not
bundle the capacitor plugins with Ionic Framework.
- Introduces a `getCapacitor()` function for interacting with the
`window.Capacitor` object through a typed object.
**How does it work?**
The idea is we want the type safety from the Capacitor packages, without
directly bundling that source code within Ionic Framework. This means we
use the Capacitor deps where a type is needed, but clone any enums where
a value is referenced. If a Capacitor dep changes the supported values,
Typescript will fail to compile and that will signal to use to update
our enum values to match any changes.
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Dev-build: `7.1.2-dev.11688696027.1c4d4ad1`
Tested against a demo app for some of the core behavior:
https://github.com/sean-perkins/capacitor-ionic-plugins-demo
Issue number: N/A
---------
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
The `angular` directory sits at the root of the project instead of in
`packages` with all the other JS Framework integrations. This does not
cause any functional issues with Ionic, but it is confusing since
integrations are not in a consistent place.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- Moves the `angular` directory to `packages/angular`
Note: Most files should remain unchanged. The only files I changed are
the files that had direct paths to the old `angular` directory:
1. Removes the `angular` path in `lerna.json`. This is now covered by
`packages/*`
2. Updated the angular file path in `.gitignore`
3. Updates the path to the angular package in `stencil.config.ts` for
the Angular Output Targets
4. Updates some of Angular's sync scripts to correctly get the core
stylesheets as well as the core package.
5. Updates the test app sync script to correctly sync core and
angular-server
~I'm not entirely sure why GitHub thinks
https://github.com/ionic-team/ionic-framework/pull/27719/files#diff-f5bba7e7c7c75426e2b9c89868310cb03890493b4efe0252adf8d12cc8398962
is a new file since it exists in `main` here:
1f06be4a31/angular/test/base/scripts/build-ionic.sh~
Fixed in
6e7fc49827
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Dev build: `7.1.2-dev.11688052109.13454f5c`
Issue number: N/A
---------
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
Stencil nightly builds are broken
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
this commit updates the name of the dependent job for building the
angular server (`build-angular-server`).
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Issue number: N/A
---------
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
`@ionic/angular-server` has a dependency on `@ionic/core` not
`@ionic/angular` (see
1f06be4a31/packages/angular-server/package.json).
As a result, once `@ionic/core` is built there is no reason for
`@ionic/angular-server` to also wait for `@ionic/angular` to build
before it can build.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- `@ionic/angular-server` now waits for `@ionic/core` to build instead
of `@ionic/angular`
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Dev build sample run:
https://github.com/ionic-team/ionic-framework/actions/runs/5414097033
Issue number: N/A
---------
## What is the current behavior?
Nightly build and release broken in #27711 due to missing shell property
in GitHub Actions workflow.
## What is the new behavior?
- Nightly build and release works.
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
## Other information
Issue number: #
---------
## What is the current behavior?
Package provenance was disabled in #27656.
## What is the new behavior?
- Package provenance is enabled.
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
## Other information
Issue number: N/A
---------
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
We have a workflow that is no longer needed (that I wrote 🙃)
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
remove the stencil v4 nightly workflow following the stencil v4 release
on 2023.06.26
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
I'll have to take a look at the Actions tab at GitHub after this is
merged. I may have to do some manual cleanup (deleting workflow runs)
once this file is no longer in the repo.
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Issue number: N/A
---------
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
[GitHub Actions comes with Lerna 7
preinstalled](https://github.com/actions/runner-images/blob/main/images/linux/Ubuntu2204-Readme.md#project-management)
which has a breaking change for `lerna bootstrap`. This is causing Dev
builds and releases to intermittently fail despite our `package.json`
wanting Lerna 5.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- Ensure that the local version of Lerna is used instead of the global
Lerna. Long term we should look to update to Lerna 7 and migrate off
`lerna bootstrap`.
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Issue number: N/A
---------
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
In https://github.com/ionic-team/ionic-framework/pull/27598, I failed to
push a change that used the correct tag in the new Stencil v4 tag
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
this commit updates the stencil-version field for the Stencil v4
workflow to use the correct tag
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
[This
log](https://github.com/ionic-team/ionic-framework/actions/runs/5190015779)
shows it being used properly:
<img width="579" alt="Screenshot 2023-06-06 at 10 53 18 AM"
src="https://github.com/ionic-team/ionic-framework/assets/1930213/eca96936-a114-4fb5-b473-cbd6bfc71b3a">
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Issue number: N/A
---------
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
we have no way to test stencil v4 as a part of the nightly build
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
this commit adds a new ci cron job, 'stencil v4 build'. the purpose of
this job is to run the ionic framework ci suite on an nightly basis,
similar to the stencil nightly build.
a separate workflow has been created, as opposed to adding a `matrix` to
the pre-existing workflow, as doing so would require a great deal of
refactoring to the existing workflow. instead, a separate workflow
allows us to simply delete the this workflow once stencil v4 is
released.
as a part of this commit, we add a new input to the
'build-core-stencil-prerelease' action, allowing users of the action to
input the npm tag of `@stencil/core` to install
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
I tested the Stencil nightly job manually -
https://github.com/ionic-team/ionic-framework/actions/runs/5180201562
I was unable to run the v4 job, as it's not in `main` (yet 😉 ). But, I
can diff it against the known-to-be-good nightly workflow. The following
shows only the name of the job & npm tag are different:
```diff
diff stencil-v4.yml stencil-nightly.yml
3c3
< name: 'Stencil v4 Build'
---
> name: 'Stencil Nightly Build'
21c21
< build-core-with-stencil-v4:
---
> build-core-with-stencil-nightly:
27c27
< stencil-version: v4-next
---
> stencil-version: nightly
30c30
< needs: [build-core-with-stencil-v4]
---
> needs: [build-core-with-stencil-nightly]
37c37
< needs: [build-core-with-stencil-v4]
---
> needs: [build-core-with-stencil-nightly]
44c44
< needs: [build-core-with-stencil-v4]
---
> needs: [build-core-with-stencil-nightly]
65c65
< needs: [build-core-with-stencil-v4]
---
> needs: [build-core-with-stencil-nightly]
93c93
< needs: [build-core-with-stencil-v4]
---
> needs: [build-core-with-stencil-nightly]
129c129
< needs: [build-core-with-stencil-v4]
---
> needs: [build-core-with-stencil-nightly]
165c165
< needs: [build-core-with-stencil-v4]
---
> needs: [build-core-with-stencil-nightly]
```
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Issue number: N/A
---------
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
PRs are first assigned to the team before being assigned to a single
team member. This causes reviews to show up
https://github.com/notifications for everyone even if they are not the
final reviewer.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- GitHub automatically assigns reviewers now, so I removed the global
Framework team codeowner. All PRs in this repo should have a team member
automatically assigned.
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Issue number: N/A
---------
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
Screenshots in a branch are currently compared against the screenshots
on `main`. While we ultimately need to compare against ground truths in
`main`, this can cause tests to seemingly fail at random. For example,
this branch was created based on
eb2772c0ce.
However,
381de0b3d3
was added to `main` after the first commit and has new screenshot diffs.
I do not have the latest ground truths for the changed screenshots in
this branch, so the screenshot tests failed in
https://github.com/ionic-team/ionic-framework/actions/runs/4995702351/jobs/8947969061
even though I did not change any component implementations.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- Test runs in CI now run against the ground truths available in the
branch. When a commit is added to the merge queue, it will be tested
against the latest `main` and any commits ahead of it in the queue. As a
result, if there _is_ a conflict then the test run will fail at that
point and the commit will be removed from the merge queue.
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Issue number: N/A
---------
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
`@stencil/core` is the only package checked by dependabot
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
this commit updates the allow list for dependabot to allow additional
stencil-scoped packages. the motivation for this is to allow the other
stencil dependencies used in ionic framework to be automatically bumped
by automation, rather than making it a manual process. this is intended
to help with the stencil v4 rollout, where new versions on stencil
packages (other than core) will be released with early v4 support.
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
I arbitrarily alphabetized the `@stencil/*` section of this allow-list.
LMK if you'd prefer something different here
Issue number: N/A
---------
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
The generator migration is complete, so we can safely remove the legacy
test infrastructure.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- Removes the legacy test infrastructure
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Note: There's still a bit of sharding imbalance that I need to look into
(test runner 17 is a few minutes longer than all the other test
runners), but I will handle that in a separate PR.
Issue number: N/A
---------
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
The text appears too cramped. I'd like there to be more space between
each paragraph.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- Increase spacing between paragraphs
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Issue number: N/A
---------
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
The team would like to provide more context surrounding the `community
feedback wanted` label in terms of setting expectations that we may
accept or reject the feedback after a community feedback period.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- Ionitron will now comment on `community feedback wanted` issues.
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Issue number: FW-4092
---------
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- Updates the Angular 16 test app to the official dependencies
- Removes the legacy peer dependency install flag (needed during the RC
cycle)
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Issue number: N/A
---------
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
`set-output` usage is deprecated in favor of environment files.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- Updates the `test-core-screenshot` workflow to remove `set-output` in
favor of environment files.
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Removes the comments about reading the contributing guide from our pull
request template. GitHub recommends the contributing guide in a popup to
new contributors, and it also recommends it if anything has changed
recently.
Issue number: #
---------
<!-- Please refer to our contributing documentation for any questions on
submitting a pull request, or let us know here if you need any help:
https://ionicframework.com/docs/building/contributing -->
<!-- Some docs updates need to be made in the `ionic-docs` repo, in a
separate PR. See
https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#modifying-documentation
for details. -->
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
the legacy screenshot test verification step can fail due to improper
configuration
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
this commit updates the name of the legacy screenshot test so that we
properly gate on verifying that the legacy tests passed
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
I tested this by kicking off the workflow -
https://github.com/ionic-team/ionic-framework/actions/runs/4830665737
Yesterday, I [put up a
PR](https://github.com/ionic-team/ionic-framework/pull/27298) that did
the same. However, I failed to verify it succeeded (since we don't gate
on Stencil nightly) - only that it started (since that was the point of
failure yesterday).
Today, I have verified that it passes all the way through
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Issue number: N/A
---------
<!-- Please refer to our contributing documentation for any questions on
submitting a pull request, or let us know here if you need any help:
https://ionicframework.com/docs/building/contributing -->
<!-- Some docs updates need to be made in the `ionic-docs` repo, in a
separate PR. See
https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#modifying-documentation
for details. -->
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
The Stencil nightly job fails with the following error:
```
The workflow is not valid. .github/workflows/stencil-nightly.yml (Line: 101, Col: 13): Job 'test-core-screenshot-legacy' depends on unknown job 'build-core'.
.github/workflows/stencil-nightly.yml (Line: 122, Col: 12): Job 'verify-screenshots-legacy' depends on job 'test-core-screenshot-legacy' which creates a cycle in the dependency graph.
```
This started to fail last night. I believe this is a result of
https://github.com/ionic-team/ionic-framework/pull/27228, specifically
this line
4fe8de7df7 (diff-2f087e8fac034d51c50fc9350e10ceb4034298c16dd0d4f414f79d88ebb71aa0R101)
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
this commit updates the job dependency hierarchy for
test-core-screenshot-legacy from `build-core` to
`build-core-with-stencil-nightly`. the former is not a valid job name
for the `stencil-nightly` workflow, likely just a copy/paste error
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
To test this, I was able to kick off a version of this job with this
branch:
https://github.com/ionic-team/ionic-framework/actions/runs/4819613134
and saw the new/legacy screenshot tests running:
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Issue number: N/A
---------
This PR adds documentation for the following:
1. A step-by-step guide for migrating to generators.
2. Documentation on the custom tooling we built on top of Playwright
3. A best practice guide. Note: This was originally in the Ionic
internal Notion, but I moved it here because it is of value to community
contributors.
I also added a few new best practices that were not on the Notion guide:
- Components must have a `basic` test with an `index.html` file
- Slow tests should be broken up across multiple files for CI
performance
- Rendering and functionality tests should be broken up across multiple
test.describe blocks or files
---------
Co-authored-by: Amanda Johnston <90629384+amandaejohnston@users.noreply.github.com>
Issue number: N/A
---------
<!-- Please refer to our contributing documentation for any questions on
submitting a pull request, or let us know here if you need any help:
https://ionicframework.com/docs/building/contributing -->
<!-- Some docs updates need to be made in the `ionic-docs` repo, in a
separate PR. See
https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#modifying-documentation
for details. -->
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
This is the first PR to introduce the infrastructure required to add
test generators to the Ionic Framework project. This PR introduces the
file name changes necessary to support two playwright configs, so I
recommend reviewing the PR by commit:
1e5012cea1
- Created a `playwright.config-legacy.ts` file and updates
`package.json`.
- Running `npm run test.e2e` will run the generator tests, and running
`npm run test.e2e.legacy` will run the legacy tests.
4fe8de7df7
- Updates the GitHub Action scripts to run both the modern and legacy
E2E tests. I added command modifiers to avoid collisions with output
directories.
e8bcfaf926
- Updates `*.e2e.ts` files to have the legacy format name:
`*.e2e-legacy.ts`. This naming scheme is required for the two Playwright
configs to pull in the correct files. When migrating tests to
generators, team members will rename the file to remove the `-legacy`
part.
5bf196c36d
(warning: lots of files!)
- Updates the `*.e2e.ts-snapshots` directories to have the legacy format
name: `*.e2e-legacy.ts-snapshots`. The screenshot directory in
Playwright is generated based on the test file name which is why we are
updating the screenshot directory. When migrating tests to generators,
team members will rename the directory to remove the `-legacy` part.
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Issue number: N/A
---------
<!-- Please refer to our contributing documentation for any questions on
submitting a pull request, or let us know here if you need any help:
https://ionicframework.com/docs/building/contributing -->
<!-- Some docs updates need to be made in the `ionic-docs` repo, in a
separate PR. See
https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#modifying-documentation
for details. -->
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
there are a few outdated links to a dead heroku app in the repo
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
this commit removes old links within the repo that point to a heroku app
that is no longer working/maintained. the original intent was to update
stencil-related links initially (as stencil still focuses on slack for
community communications). however, there were two references to the
ionic worldwide slack, which has been since replaced with discord. i've
updated those link (which were only in comments) as well. after applying
this commit, there are no references to 'heroku' in the repo
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Feel free to reject this/request we remove these links altogether. I'm
not entirely sure we want to continue to provide them in the `core/` and
`angular/` directories (when we don't have them in `packages/react/` nor
`packages/vue/`
Issue number: N/A
---------
<!-- Please refer to our contributing documentation for any questions on
submitting a pull request, or let us know here if you need any help:
https://ionicframework.com/docs/building/contributing -->
<!-- Some docs updates need to be made in the `ionic-docs` repo, in a
separate PR. See
https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#modifying-documentation
for details. -->
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
Original Discussion:
https://github.com/ionic-team/stencil-ds-output-targets/pull/327#discussion_r1176950960
This flag was left over from our migration from custom build scripts to
Lerna. Since we use `npm ci` the `--no-package-lock` flag is not needed.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- Removes `--no-package-lock` from build scripts.
- Here is an example test run of a dev build succeeding without the
`--no-package-lock` flag:
https://github.com/ionic-team/ionic-framework/actions/runs/4801503002
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
<!-- Please refer to our contributing documentation for any questions on
submitting a pull request, or let us know here if you need any help:
https://ionicframework.com/docs/building/contributing -->
<!-- Some docs updates need to be made in the `ionic-docs` repo, in a
separate PR. See
https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#modifying-documentation
for details. -->
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
<!-- Issues are required for both bug fixes and features. -->
Issue URL: Internal
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- Adds Angular 16 test app
- Introduces version test to validate which major version of Angular is
being tested
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
Now that the PR description is used as the default commit message, the
issue a PR resolves is specified directly in the PR description.
However, the conventional-commits tool isn't smart enough to recognize
full issue URLs, only the issue number (e.g. `resolves #12345`). This PR
updates the template to nudge the description in this direction.
Now that the default commit message uses the PR description, the
conventional commit tool is getting confused when it sees other URLs and
text that looks like issues (like hex colors).
Using the `-------` separates the issue URL from the rest of the PR
description and helps the conventional commits tool generate the
changelog correctly.
Example:
https://github.com/ionic-team/ionic-framework/releases/tag/v7.0.3-nightly.20230417
`item: ios active state has correct contrast` has issue 000 as the issue
the commit closes. This is happening because I mentioned the 000 hex
color in the commit description.
updates the contributing guide to:
- include steps to package changes and test them in an external app
- fix the references to the test apps so they point to the right
directories
- link to the test app directories for steps to run them and preview
changes from within this repository
<!-- Please refer to our contributing documentation for any questions on
submitting a pull request, or let us know here if you need any help:
https://ionicframework.com/docs/building/contributing -->
<!-- Some docs updates need to be made in the `ionic-docs` repo, in a
separate PR. See
https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#modifying-documentation
for details. -->
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
<!-- Issues are required for both bug fixes and features. -->
Unbuilt changes in `core` are not always being caught because the
`api.txt` file is not cached.
Stencil will potentially update both the `src/components.d.ts` and
`api.txt` files when there are built changes to the public API. We need
to check both files when accounting for unbuilt changes.
Example: https://github.com/ionic-team/ionic-framework/pull/25858
This had updates to `api.txt` that were not built.
Built changes PR:
https://github.com/ionic-team/ionic-framework/pull/25933
Example of this change catching `api.txt` changes as intended:
https://github.com/ionic-team/ionic-framework/pull/27165
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- GitHub Actions core build step caches `api.txt`
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
