adobe / aem-core-wcm-components

Standardized components to build websites with AEM.
https://docs.adobe.com/content/help/en/experience-manager-core-components/using/introduction.html
Apache License 2.0
733 stars 743 forks source link

[DevTools] Add aemfed npm module for live upload / reload of content changes #300

Closed richardhand closed 5 years ago

richardhand commented 5 years ago

Add an npm script to allow running aemfed, for live upload and reload of templates, client libraries and resources during development. https://github.com/abmaonline/aemfed.

abmaonline commented 5 years ago

@richardhand thanks for mentioning aemfed and creating a PR for it :+1: It can be a real lifesaver when working with these OG AEM components.

Too bad it has been reverted πŸ˜‰ Probably because it was more of a personal preference, instead of a tool to maintain the quality/consistency of the project, like the other devDependencies.

But please, spread the love (or give the repo a star ⭐️ 😎)!

vladbailescu commented 5 years ago

@abmaonline The only reason we reverted the PR was because builds were taking too long (to me it seems it was due to aem-sync and less-tree dependencies pointing to specific commits). Could you help us to sort this out so we can reapply the PR?

abmaonline commented 5 years ago

@vladbailescu thanks for the update πŸ‘ I will try the tarball solution described in https://coderwall.com/p/q_gh-w/fork-and-patch-npm-moduels-hosted-on-github to see if it works and helps in the build times.

Do you have the exact build statement available, so I can see if the change makes any difference before pushing?

vladbailescu commented 5 years ago

The npm install step (running with the maven frontend plugin) is taking very long, ex:

07:29:15 [INFO] Running 'npm install' in /apps/jenkins/workspace/github-aem-core-wcm-components/corecmp/content 07:42:22 [WARNING] npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fsevents@1.2.4 (node_modules/fsevents):

When I ran npm install manually it got stuck a lot at:

βΈ¨ β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘βΈ© ⠏ fetchMetadata: sill resolveWithNewModule aemfed@0.0.5 checking installable status

vladbailescu commented 5 years ago

Created a new PR at https://github.com/Adobe-Marketing-Cloud/aem-core-wcm-components/pull/339

Strangely, on Travis CI it was not taking this long. @abmaonline Can you try and build from that branch?

abmaonline commented 5 years ago

Running the build locally Did try the branch locally and noticed a few things:

First ran npm install in /content/ using nvm with node v6.14.4 and npm v6.4.1 and the following statement did indeed take a few seconds:

βΈ¨ β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘β–‘βΈ© ⠏ fetchMetadata: sill resolveWithNewModule aemfed@0.0.5 checking installable status

But during the second run it wasn't noticeable anymore (probably because internal npm caching).

Then I tried to run mvn clean install from the root of the project and also got warnings like these (but much faster):

07:29:15 [INFO] Running 'npm install' in /apps/jenkins/workspace/github-aem-core-wcm-components/corecmp/content 07:42:22 [WARNING] npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fsevents@1.2.4 (node_modules/fsevents):

Ending in a npm error about not being able to find the files in the warnings and aborting the build.

It turned out the frontend-maven-plugin was using npm 5.6.0 (and node 8.10.0) which probably got confused by the node_modules folder initially filled by npm 6.4.1 (and node 6.14.4). After I removed the /content/node_modules/ folder and ran mvn clean install again, the build was successful (w/o noticeable delays).

Since package.json specifies a dependency on npm ^6.4.1 and the frontend-maven-plugin is using npm 5.6.0, I can see how this can cause problems when running the build locally in both ways (also explains the constant changes in the package-lock.json, since it is regenerated by different versions of npm all the time).

Not sure how this has effect on the jenkins builds, since it probably only used the frontend-maven-plugin? I've had some issues in the past with jenkins jobs using old modules and probably the folks at Travis CI fixed this, leading to the different results?

Updating aemfed to use tarballs Also created release v0.0.7 for aemfed to use a tarball from git instead of the complete repo itself. Should be faster and doesn't need git on the system performing the npm install. It also fixes the npm audit warnings.

To see if it has any effect on the build, I created a commit with the updated version of aemfed on the dev/aemfed branch in my fork: abmaonline/aem-core-wcm-components#c2cd6d3 Do you want to apply the changes on your branch yourself, a PR on your branch or a new PR directly on develop?

richardhand commented 5 years ago

Thanks @abmaonline for the feedback! I applied some changes directly to the branch @vladbailescu created at https://github.com/Adobe-Marketing-Cloud/aem-core-wcm-components/pull/339 so we can test improvements to the build time.

richardhand commented 5 years ago

No noticeable delays for me with via mvn clean install or npm install following the change. @vladbailescu - can you confirm?

vladbailescu commented 5 years ago

Yes, no delays anymore for me either.

vladbailescu commented 5 years ago

Seems to be working well on Jenkins as well.

vladbailescu commented 5 years ago

Closed through #339 and https://github.com/Adobe-Marketing-Cloud/aem-core-wcm-components/commit/38300d9a9b2abf646e72821cbbe870a8b07dc26d