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.

Sign up for GitHub

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

Jump to bottom

chore: update to TypeScript 5, Stencil 4 (and more :finnadie: ) #3162

Merged
merged 34 commits into from
Sep 12, 2023
Merged

chore: update to TypeScript 5, Stencil 4 (and more :finnadie: ) #3162

merged 34 commits into from
Sep 12, 2023

Conversation

louis-bompart
Copy link
Collaborator

@louis-bompart louis-bompart commented Sep 9, 2023 •
edited
Loading

  • Rebase/clean the commit history

Mostly upgrade Stencil to v4 and TypeScript to v5.

Bonuses:

  • Fix Angular E2E unstable
  • Jest 29 in most of the project (exclusion on Atomic)

The riskiest/biggest change is around the patch in the Stencil compiler (good old AST :finnadie:) See:

Will scope out the type fix of the code-editor component of Storybook into an improvement: Unknown amount of effort, very low reward.

https://coveord.atlassian.net/browse/KIT-2312

@github-actions
Copy link

github-actions bot commented Sep 9, 2023 •
edited
Loading

Pull Request Report

PR Title

✅ Title follows the conventional commit spec.

Bundle Size

File Old (kb) New (kb) Change (%)
case-assist 182.1 182.1 0
commerce 264.2 264.2 0
search 345.4 345.4 0
insight 299 299 0
product-listing 287.7 287.7 0
product-recommendation 156.3 156.3 0
recommendation 192.9 192.9 0
ssr 207.5 207.5 0

@louis-bompart louis-bompart changed the base branch from stencil-4 to master September 11, 2023 00:45
@louis-bompart louis-bompart changed the title typescript 5 stencil 4, typescript 5 Sep 11, 2023
@github-actions
Copy link

github-actions bot commented Sep 11, 2023 •
edited
Loading

Dependency Review

The following issues were found:
  • ✅ 0 vulnerable package(s)
  • ❌ 1 package(s) with incompatible licenses
  • ❌ 1 package(s) with invalid SPDX license definitions
  • ⚠️ 11 package(s) with unknown licenses.
See the Details below.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA 8b5aa97.
Ensure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice.

License Issues

package-lock.json

PackageVersionLicenseIssue Type
axe-core4.8.1MPL-2.0Incompatible License
esprima1.2.2BSDInvalid SPDX License
@salesforce-ux/design-system2.22.0NullUnknown License
@typescript-eslint/scope-manager5.62.0NullUnknown License
@typescript-eslint/types5.62.0NullUnknown License
@typescript-eslint/typescript-estree5.62.0NullUnknown License
@typescript-eslint/utils5.62.0NullUnknown License
@typescript-eslint/visitor-keys5.62.0NullUnknown License

utils/atomic-storybook/package-lock.json

PackageVersionLicenseIssue Type
string-width-cjs4.2.3NullUnknown License
strip-ansi-cjs6.0.1NullUnknown License
wrap-ansi-cjs7.0.0NullUnknown License

utils/atomic-storybook/package.json

PackageVersionLicenseIssue Type
@types/lodash^4.14.198NullUnknown License
@types/react^18.2.21NullUnknown License
Allowed Licenses: 0BSD, Apache-2.0, Apache-2.0 AND MIT, Apache-2.0 AND BSD-3-Clause AND Python-2.0, Beerware, BlueOak-1.0.0, BSD-1-Clause, BSD-2-Clause, BSD-2-Clause-Patent, BSD-2-Clause-Views, BSD-2-Clause AND MIT, BSD-3-Clause, BSD-3-Clause-Attribution, BSD-3-Clause-Clear, BSL-1.0, CC-BY-3.0, CC-BY-4.0, CC0-1.0, CNRI-Python, curl, HPND, IBM-pibs, ImageMagick, ISC, JSON, MIT, MIT-0, MIT AND ISC, MIT AND Python-2.0, MIT-advertising, mpi-permissive, NCSA, ODC-By-1.0, PDDL-1.0, Plexus, PostgreSQL, PSF-2.0, Python-2.0, Python-2.0.1, SAX-PD, Unlicense, UPL-1.0, W3C, Wsuipa, WTFPL, X11, X11-distribute-modifications-variant, Xerox, Zlib, ZPL-2.1

Scanned Manifest Files

package-lock.json
package.json
packages/atomic-angular/package.json
packages/atomic-hosted-page/package.json
packages/atomic/package.json
packages/auth/package.json
packages/bueno/package.json
packages/headless/package.json
packages/quantic/package.json
packages/samples/angular/package.json
packages/samples/atomic-next/package.json
packages/samples/atomic-react/package.json
packages/samples/headless-react/package.json
packages/samples/stencil/package.json
packages/samples/vuejs/package.json
utils/atomic-storybook/package-lock.json
utils/atomic-storybook/package.json
utils/nx-plugin/package.json
utils/release/package.json

@louis-bompart louis-bompart marked this pull request as ready for review September 11, 2023 05:08
@louis-bompart louis-bompart requested review from a team as code owners September 11, 2023 05:08
btaillon-coveo
btaillon-coveo approved these changes Sep 11, 2023
View reviewed changes
Copy link
Contributor

@btaillon-coveo btaillon-coveo left a comment

Choose a reason for hiding this comment

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

yes!

.../atomic/src/components/search/result-template-components/result-template-decorators.spec.tsx Outdated Show resolved Hide resolved
utils/atomic-storybook/.storybook/code-sample-addon/code-sample-panel.jsx Show resolved Hide resolved
utils/atomic-storybook/.storybook/register.js Show resolved Hide resolved
dmbrooke
dmbrooke approved these changes Sep 11, 2023
View reviewed changes
Copy link
Contributor

@dmbrooke dmbrooke left a comment

Choose a reason for hiding this comment

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

A couple of questions, but this looks good 🙂

.../atomic/src/components/search/result-template-components/result-template-decorators.spec.tsx Outdated Show resolved Hide resolved
packages/headless-react/jest.config.js Show resolved Hide resolved
olamothe
olamothe approved these changes Sep 11, 2023
View reviewed changes
Copy link
Member

@olamothe olamothe left a comment

Choose a reason for hiding this comment

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

That's nice !

packages/atomic-angular/package.json Outdated Show resolved Hide resolved
.../atomic/src/components/search/result-template-components/result-template-decorators.spec.tsx Outdated Show resolved Hide resolved
utils/atomic-storybook/.storybook/code-sample-addon/code-sample-panel.jsx Show resolved Hide resolved
@olamothe
Copy link
Member

I'll checkout the branch and build storybook locally to do some manual/smoke tests with it.
People use it quite a lot when implementing, I wanna make sure everything is still good on that front.

Everything seems 💯

🚢

@louis-bompart
Copy link
Collaborator Author

Dependency Review

The following issues were found:

  • ❌ 2 vulnerable package(s)
  • ❌ 1 package(s) with incompatible licenses
  • ❌ 2 package(s) with invalid SPDX license definitions
  • ⚠️ 10 package(s) with unknown licenses.

See the Details below.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA b86cecd.

Ensure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice.

Vulnerabilities

package-lock.json

Name Version Vulnerability Severity
json5 2.2.1 Prototype Pollution in JSON5 via Parse Method high
json5 1.0.1 Prototype Pollution in JSON5 via Parse Method high

Only included vulnerabilities with severity high or higher.

License Issues

package-lock.json

Package Version License Issue Type
axe-core 4.8.1 MPL-2.0 Incompatible License
esprima 1.2.2 BSD Invalid SPDX License
gitconfiglocal 1.0.0 BSD Invalid SPDX License
@typescript-eslint/scope-manager 5.62.0 Null Unknown License
@typescript-eslint/types 5.62.0 Null Unknown License
@typescript-eslint/typescript-estree 5.62.0 Null Unknown License
@typescript-eslint/utils 5.62.0 Null Unknown License
@typescript-eslint/visitor-keys 5.62.0 Null Unknown License

utils/atomic-storybook/package-lock.json

Package Version License Issue Type
string-width-cjs 4.2.3 Null Unknown License
strip-ansi-cjs 6.0.1 Null Unknown License
wrap-ansi-cjs 7.0.0 Null Unknown License

utils/atomic-storybook/package.json

Package Version License Issue Type
@types/lodash ^4.14.198 Null Unknown License
@types/react ^18.2.21 Null Unknown License

Allowed Licenses: 0BSD, Apache-2.0, Apache-2.0 AND MIT, Apache-2.0 AND BSD-3-Clause AND Python-2.0, Beerware, BlueOak-1.0.0, BSD-1-Clause, BSD-2-Clause, BSD-2-Clause-Patent, BSD-2-Clause-Views, BSD-2-Clause AND MIT, BSD-3-Clause, BSD-3-Clause-Attribution, BSD-3-Clause-Clear, BSL-1.0, CC-BY-3.0, CC-BY-4.0, CC0-1.0, CNRI-Python, curl, HPND, IBM-pibs, ImageMagick, ISC, JSON, MIT, MIT-0, MIT AND ISC, MIT AND Python-2.0, MIT-advertising, mpi-permissive, NCSA, ODC-By-1.0, PDDL-1.0, Plexus, PostgreSQL, PSF-2.0, Python-2.0, Python-2.0.1, SAX-PD, Unlicense, UPL-1.0, W3C, Wsuipa, WTFPL, X11, X11-distribute-modifications-variant, Xerox, Zlib, ZPL-2.1

Scanned Manifest Files

package-lock.json
package.json
packages/atomic-angular/package.json
packages/atomic-hosted-page/package.json
packages/atomic/package.json
packages/auth/package.json
packages/bueno/package.json
packages/headless/package.json
packages/quantic/package.json
packages/samples/angular/package.json
packages/samples/atomic-next/package.json
packages/samples/atomic-react/package.json
packages/samples/headless-react/package.json
packages/samples/stencil/package.json
packages/samples/vuejs/package.json
utils/atomic-storybook/package-lock.json
utils/atomic-storybook/package.json
utils/nx-plugin/package.json
utils/release/package.json

Verified:

  • Licenses issues have already been addressed in the past (BSD is fine, MPL is OK here 'cause dev dep)
  • vuln on JSON5 are OK because there's solely on SFDX-CLI

@louis-bompart
Copy link
Collaborator Author

ionic-team/stencil#3831 (comment) There might be some side-effects with the patch, waiting for confirmation tho: our tests seems to pass, so I want to see what would break

@louis-bompart
Copy link
Collaborator Author

ionic-team/stencil#3831 (comment) There might be some side-effects with the patch, waiting for confirmation tho: our tests seems to pass, so I want to see what would break

After discussion with both stencil maintainer & the team, we decided to go ahead with the patch/custom behavior and introduce a caveat: 8b5aa97

SimonMilord
SimonMilord approved these changes Sep 12, 2023
View reviewed changes
Copy link
Contributor

@SimonMilord SimonMilord left a comment

Choose a reason for hiding this comment

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

👍

@louis-bompart louis-bompart merged commit afabd13 into master Sep 12, 2023
31 of 32 checks passed
@louis-bompart louis-bompart deleted the typescript-5 branch September 12, 2023 19:06
@louis-bompart louis-bompart mentioned this pull request Sep 13, 2023
Closed
@alexprudhomme alexprudhomme mentioned this pull request Sep 14, 2023
Merged
@fbeaudoincoveo fbeaudoincoveo changed the title chore: update to TypeScript 5, Stencil 4 (and more :finnadie: ) chore: update to TypeScript 5, Stencil 4 (and more :finnadie: ) KIT-2312 Sep 21, 2023
@fbeaudoincoveo fbeaudoincoveo changed the title chore: update to TypeScript 5, Stencil 4 (and more :finnadie: ) KIT-2312 chore: update to TypeScript 5, Stencil 4 (and more :finnadie: ) Sep 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SimonMilord SimonMilord SimonMilord approved these changes

@btaillon-coveo btaillon-coveo btaillon-coveo approved these changes

@dmbrooke dmbrooke dmbrooke approved these changes

@olamothe olamothe olamothe approved these changes

@y-lakhdar y-lakhdar Awaiting requested review from y-lakhdar y-lakhdar is a code owner automatically assigned from coveo/search

@mrrajamanickam-coveo mrrajamanickam-coveo Awaiting requested review from mrrajamanickam-coveo mrrajamanickam-coveo is a code owner automatically assigned from coveo/search

@erocheleau erocheleau Awaiting requested review from erocheleau erocheleau is a code owner automatically assigned from coveo/salesforce-integration

@mmitiche mmitiche Awaiting requested review from mmitiche mmitiche is a code owner automatically assigned from coveo/salesforce-integration

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@louis-bompart @olamothe @dmbrooke @btaillon-coveo @SimonMilord