Skip to content

Conversation

@trevoring-okta
Copy link
Contributor

@trevoring-okta trevoring-okta commented Jan 12, 2024

Description:

  • Uses patch-package to patch Preact bug with the same fix found here
  • Adds support for passing a SKIP_POSTINSTALL flag to a yarn install call which prevents the postinstall patch-package call from running. This is needed for the verify-package.sh script to work because the yarn install calls from that script were coming from the installation of the SIW as a dependency for a test app, which was breaking since the test app itself did not have patch-package installed. The english leak test suite doesn't need patch-package anywhere except for yarn install calls from the package root
  • Other small lint fixes

PR Checklist

Issue:

Reviewers:

Screenshot/Video:

Highlighted div with "Label" is imported from Odyssey and was passed translate="no" as a prop

Screenshot 2024-01-12 at 6 43 17 PM

Downstream Monolith Build:

package.json Outdated
"test:parity-ci": "OKTA_SIW_SKIP_FLAKY=true yarn codegen && run-p -r test:parity-setup test:parity-run",
"test:parity-ci-flaky": "OKTA_SIW_GEN3=true OKTA_SIW_ONLY_FLAKY=true yarn test:parity-ci"
"test:parity-ci-flaky": "OKTA_SIW_GEN3=true OKTA_SIW_ONLY_FLAKY=true yarn test:parity-ci",
"postinstall": "if [[ -z \"$SKIP_POSTINSTALL\" ]]; then patch-package ; fi"
Copy link
Contributor

Choose a reason for hiding this comment

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

This command would not work on Windows.
Tried with GitBash and received error "-z was unexpected at this time."

Maybe we can just strip out postinstall command from npm package here (since we don't export Gen3 anyway in npm package, and I hope this fix is temporary)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ooh, didn't know about this prepack routine. Good idea, will look into it

Copy link
Contributor

@denysoblohin-okta denysoblohin-okta left a comment

Choose a reason for hiding this comment

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

Please rebase to the latest odyssey-1.x commit.
Should resolve translate="yes" issues in snapshots

@trevoring-okta trevoring-okta force-pushed the ti-OKTA-684531-patch-preact-translate branch from c1ceca9 to 7cbfa55 Compare January 17, 2024 16:00
@trevoring-okta trevoring-okta merged commit 3e32e6a into odyssey-1.x Jan 17, 2024
trevoring-okta added a commit that referenced this pull request Feb 12, 2024
* Patch preact translate bug with patch-package
trevoring-okta added a commit that referenced this pull request Feb 13, 2024
* Patch preact translate bug with patch-package
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.

3 participants