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.

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

Angular app adaptation fails #850

Closed
1 of 4 tasks
0xAnakin opened this issue Feb 10, 2022 · 9 comments
Closed
1 of 4 tasks

Angular app adaptation fails #850

0xAnakin opened this issue Feb 10, 2022 · 9 comments
Assignees

Comments

@0xAnakin
Copy link

0xAnakin commented Feb 10, 2022

Tried to adapt an angular app and it failed firstly when using npm as a dependency manager see #848 and after that when trying to locate the path build/static/js which does not exist in an out of the box angular app. Use the exact node version mentioned below and npm as a package manager. See reproduction steps listed at the end.

  • 🤔 Question
  • 🐛 Bug report
  • 🎁 Feature request
  • 🤷‍♀️ Other

node version

  • v16.14.0

packages installed globally

├── @angular/[email protected]
├── [email protected]
├── [email protected]
├── [email protected]
└── [email protected]

Steps

  • install nodejs LTS (16.14.0)
  • type & hit enter: npm install -g yo generator-liferay-js @angular/cli
  • type & hit enter: ng new angular-app
  • type & hit enter: cd angular-app
  • type & hit enter: yo liferay-js:adapt
@izaera izaera self-assigned this Feb 11, 2022
@izaera
Copy link
Member

izaera commented Feb 11, 2022

Hey @0xAnakin , can you try to use the new liferay-cli tool to adapt your project? Sooner than later we will deprecate the yeoman generator and the liferay-cli tech stack is leaner and simpler so it should be less error prone.

I tested it yesterday for this version of @angular/cli 👉

and it worked perfectly, so that should work for you too.

You can check liferay-cli documentation here 👉 https://github.com/liferay/liferay-frontend-projects/blob/master/projects/js-toolkit/docs/manuals/liferay-cli.md

If for some reason you need to keep using the old stack (yeoman generator) I think you are experiencing the same problem(s) listed here -> #613

@0xAnakin
Copy link
Author

0xAnakin commented Feb 11, 2022

Hey @izaera I had no idea you had a new tool out 😮. I tried with a new sample angular app and I get the following error about a .liferay.json file missing?

image

@0xAnakin
Copy link
Author

0xAnakin commented Feb 11, 2022

I created a .liferay.json myself and added the following:

{
	"liferayDir": "C:/Development/liferay/liferay-dxp-7.3.10.1-sp1-ethniki"
}

Then I tried npm run deploy:liferay but I got an error that it requires building first, so I did npm run build:liferay and then npm run deploy:liferay and it worked.

I assume that:

  • the file should be automatically created
  • deploy:liferay should also build and not just move the jar file

@izaera
Copy link
Member

izaera commented Feb 14, 2022

· the file should be automatically created

Yep. That one was working, so it probably broke with the last update :-(.

· deploy:liferay should also build and not just move the jar file

Not so sure about that. I know it did that in previous versions of the JS Toolkit, and I know it's handy, but on the other side, building usually tooks very long and it isn't always needed before deployment... 🤔

We may unify those targets again in the future, when we optimize the build times, but for now, I feel it is better to separate them...

In any case, I'd like to hear more opinions on this, so I'm mentioning some more people to see what they think: @bryceosterhaus @pablo-agulla @dsanz @kresimir-coko

@0xAnakin
Copy link
Author

bump

@dsanz
Copy link
Member

dsanz commented Feb 23, 2022

I know it did that in previous versions of the JS Toolkit, and I know it's handy, but on the other side, building usually tooks very long and it isn't always needed before deployment

This is handy yeah, but at the same time, forces a build for each deployment, so if you want to deploy to more than one target, that will slow down things. Couldn't this be somehow configurable with a sensible default? For simple use cases it may make sense to tie both tasks. Maybe having a separate entry under scripts?

@izaera
Copy link
Member

izaera commented Feb 24, 2022

I like the idea of having two entries on the package.json 👍

How about:

  • deploy : to build & deploy
  • deploy:only: to only deploy

Or is it better to have the opposite? 🤔 :

  • build-deploy : to build & deploy
  • deploy: to only deploy

@pablo-agulla
Copy link

For me it's more clear the second option, however if in the portal, deploy always builds & deploys, I'll go for option 1

@izaera
Copy link
Member

izaera commented Apr 11, 2022

Hey, I'm not going to release a new version specifically for this, but if you need it @0xAnakin just ask for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants