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

[create-sitecore-jss] Improve handling of proxyAppDestination #1868

Merged
merged 25 commits into from
Aug 6, 2024
Merged

[create-sitecore-jss] Improve handling of proxyAppDestination #1868

merged 25 commits into from
Aug 6, 2024

Conversation

art-alexeyenko
Copy link
Contributor

@art-alexeyenko art-alexeyenko commented Jul 30, 2024 •
edited
Loading

Description / Motivation

Fixes several problems with proxyAppDestination:

  • proxyAppDestination will be populated correctly if initializers set by other initializers contain proxy template
  • relative path value for proxy app is now available in helper as helper.relativeProxyAppDestination. The path will be relative to main app.

Also, to improve the new app+proxy journey:

  • Support multiple appNames for nextSteps

Testing Details

  • Unit Test Added
  • Manual Test/Other (Please elaborate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

@art-alexeyenko art-alexeyenko requested a review from a team July 30, 2024 23:51
Base automatically changed from feature/angular-xmc-proxy to dev July 31, 2024 09:32
yavorsk
yavorsk requested changes Aug 1, 2024
View reviewed changes
Copy link
Contributor

@yavorsk yavorsk left a comment

Choose a reason for hiding this comment

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

few comments, otherwise looks good to me :)

packages/create-sitecore-jss/src/common/processes/transform.ts Outdated Show resolved Hide resolved
packages/create-sitecore-jss/src/common/utils/helpers.ts Show resolved Hide resolved
illiakovalenko
illiakovalenko requested changes Aug 1, 2024
View reviewed changes
packages/create-sitecore-jss/src/common/processes/transform.ts Outdated Show resolved Hide resolved
packages/create-sitecore-jss/src/templates/angular-xmcloud/package.json Outdated Show resolved Hide resolved
packages/create-sitecore-jss/src/common/utils/helpers.ts Outdated Show resolved Hide resolved
packages/create-sitecore-jss/src/init-runner.ts Outdated Show resolved Hide resolved
packages/create-sitecore-jss/src/bin.ts Show resolved Hide resolved
packages/create-sitecore-jss/src/common/processes/transform.ts Outdated Show resolved Hide resolved
packages/create-sitecore-jss/src/common/utils/helpers.ts Show resolved Hide resolved
packages/create-sitecore-jss/src/common/utils/helpers.ts Show resolved Hide resolved
packages/create-sitecore-jss/src/common/utils/helpers.ts Show resolved Hide resolved
packages/create-sitecore-jss/src/initializers/angular/prompts.ts Outdated Show resolved Hide resolved
@illiakovalenko
Copy link
Contributor

@art-alexeyenko Let's make the naming of some variables, comments a bit more explicit. For now it's vague and means that we can use both "node-headless-ssr-proxy" and "node-xmcloud-proxy" in this journey

@art-alexeyenko
Copy link
Contributor Author

Let's make the naming of some variables, comments a bit more explicit. For now it's vague and means that we can use both "node-headless-ssr-proxy" and "node-xmcloud-proxy" in this journey

@illiakovalenko that's the idea, yes. You should be able to install node-xmcloud-proxy alongside angualar-xmcloud, node-headless-proxy alognside react, vue, angular-sxp etc, and any other proxy alongside another main app, if you need to, without running CLI command twice. It's more convenient that way.

illiakovalenko
illiakovalenko requested changes Aug 2, 2024
View reviewed changes
packages/create-sitecore-jss/src/common/prompts/sxp.ts Outdated Show resolved Hide resolved
packages/create-sitecore-jss/src/common/prompts/sxp.ts Show resolved Hide resolved
packages/create-sitecore-jss/src/common/prompts/sxp.ts Outdated Show resolved Hide resolved
packages/create-sitecore-jss/src/common/processes/next.ts Outdated Show resolved Hide resolved
packages/create-sitecore-jss/src/init-runner.ts Show resolved Hide resolved
packages/create-sitecore-jss/src/init-runner.ts Show resolved Hide resolved
packages/create-sitecore-jss/src/common/utils/helpers.ts Show resolved Hide resolved
packages/create-sitecore-jss/src/initializers/angular/prompts.ts Outdated Show resolved Hide resolved
packages/create-sitecore-jss/src/templates/angular-xmcloud/package.json Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this file change be reverted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure. Files are identical but git thinks it was changed. Outside of gitignore I don't know what can be done

Copy link
Contributor

@illiakovalenko illiakovalenko Aug 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@art-alexeyenko It's related to CRLF vs LF EOL Sequence, we are using CRLF, you need to update your settings to use CRLF as well (I suppose)
I've already pushed the change and reverted package.json

art-alexeyenko and others added 3 commits August 2, 2024 10:20
@art-alexeyenko @illiakovalenko
Update packages/create-sitecore-jss/src/init-runner.ts
ccc9519 
Co-authored-by: Illia Kovalenko <23364749+illiakovalenko@users.noreply.github.com>
@art-alexeyenko
PR comments numero dos
79e4c3a
@art-alexeyenko
packagejson newline
09572f5
illiakovalenko
illiakovalenko approved these changes Aug 6, 2024
View reviewed changes
Copy link
Contributor

@illiakovalenko illiakovalenko left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good 👍
Please, resolve conficlits and feel free to merge I resolved

@illiakovalenko illiakovalenko merged commit 0842d96 into dev Aug 6, 2024
1 check was pending
@illiakovalenko illiakovalenko deleted the bug/jss-3563-relative-proxy-dest branch August 6, 2024 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@illiakovalenko illiakovalenko illiakovalenko approved these changes

@yavorsk yavorsk Awaiting requested review from yavorsk

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

Successfully merging this pull request may close these issues.
3 participants
@art-alexeyenko @illiakovalenko @yavorsk