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

fix: babel loader and buildrc #1138

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

BalrokHS
Copy link

@BalrokHS BalrokHS commented May 9, 2023

In the latest Liferay React generator, I faced the following issues, that I was able to reproduce on different OS and setups.

  • Windows 10 (Node 16)
  • MacOS (Node 16)

The issues are:

  • Not recognising the presets, although they are installed (not recognising .babelrc)
  • Dev server not starting because of the missing dependency of babel-loader.

I added the first version, that I found it was compatible. Feel free to up/downgrade the version of babel-loader.

@BalrokHS BalrokHS changed the title Fix babel loader and buildrc fix: babel loader and buildrc May 9, 2023
@izaera
Copy link
Member

izaera commented May 9, 2023

Hey @BalrokHS, first of all, thanks a lot for you contribution 🤗, however, the generator-liferay-js was discontinued long ago in favor of @liferay/cli (see the docs and especially @liferay/cli's manual for information on how to use it).

I'm not completely against merging this PR, but given that the tool is no longer maintained I wonder if it makes sense, because if it breaks anything, we will need to roll it back, as we (the owners of this project) are probably not going to fix it, which may lead to a series of merges and rollbacks until you or someone else from the community stabilizes it 😬.

So, my proposal is that you first check if you can use @liferay/cli and if it is absolutely impossible for you, we can check again whether we want to merge this, let the project be forked, or any possibility that makes sense. I would like to give you a complete reply about this PR now but I'm not in charge of the backlog of the project, so I cannot provide any authoritative answer.

In any case, let's first check if you can use @liferay/cli better... Thx.

@BalrokHS
Copy link
Author

BalrokHS commented May 9, 2023

The problem with Liferay/cli is that I can't seem to inject any of the ClayUI dependencies that are inside of the Liferay instance. Im using 7.4.3.75 through docker and Im getting the following error on dev:

[Error] TypeError: Module name, 'classnames' does not resolve to a valid URL.

Also on build.

Could you guide me in order to resolve this with Liferay Remote App CLI implementation ?

@izaera
Copy link
Member

izaera commented May 9, 2023

Strange. That should work as long as you use the correct target platform (see https://github.com/liferay/liferay-frontend-projects/blob/master/target-platforms/README.md)

@izaera
Copy link
Member

izaera commented May 9, 2023

For example, the bundler import for classnames is defined in the @liferay/portal-7.4 target platform here -> https://github.com/liferay/liferay-frontend-projects/blob/master/target-platforms/packages/portal-7.4/config.json#L78

Without having more information or a sample project it's difficult to tell what can be happening...

@BalrokHS
Copy link
Author

BalrokHS commented May 9, 2023

Im downgrading to 7.4.3.75, and I will come back with the results.

Question. In a remote app can I use a react router with a HashRouter ?

@BalrokHS
Copy link
Author

BalrokHS commented May 9, 2023

@izaera After a long series of tries, I got it to work with Liferay/portal 7.4.3.55-ga55 on Docker.

I imported the following dependencies:

  • @clayui
  • Formik
  • Installed React-Router-DOM @6 and it works beautifully with HashRouter

Also tried to build and succeeded. React router does not seem to have a problem.

(Maybe I can post my results with a guide for the community. If you agree, please show me where 😆 ).

A recommendation is to fix the documentation of Liferay portlet creation, since the top search lead to the deprecated way and can be really confusing.

Great work with the Liferay-cli.

In the case of the PR, I think it resolves some issues that exist, so in case you decide to continue to maintain this, feel free to review the changes and accept them or even change them 😄

@izaera
Copy link
Member

izaera commented May 10, 2023

@BalrokHS Great news! 🎉

Very happy that you made it work.

Let me find out if there's some way for you to post a blog entry 👍 .

Regarding the docs, are you referring to the ones in this project? The READMEs that I linked?

And for the PR, I'll ask too what we can do.

Thx.

@izaera
Copy link
Member

izaera commented May 10, 2023

@pablo-agulla @dsanz what do you think about merging this PR? Should we try it, or should we reject it and recover this notice -> ec73c65

We used to redirect people from generator-liferay-js to liferay/cli in the past, but we had to remove the message because of the Workspace (see 0662139).

Maybe it's time to recover the message (possibly hiding it when inside a workspace) and freeze this project correctly (including the README, which is a bit ambiguous).

@izaera
Copy link
Member

izaera commented May 10, 2023

Hey @BalrokHS, the canonical place to create a blog post is https://liferay.dev/. You can create an account and post your findings there 👍

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

Successfully merging this pull request may close these issues.

2 participants