chore: splitting off lib-franklin.js

[davidnuescheler]

https://main--helix-project-boilerplate--adobe.hlx.live/
vs.
https://lib-helix--helix-project-boilerplate--adobe.hlx.live/

[aem-code-sync]

Page	Score	PSI Audit	Google
/

[aem-code-sync]

Page	Score	PSI Audit	Google
/

[aem-code-sync]

Page	Score	PSI Audit	Google
/

[aem-code-sync]

Page	Score	PSI Audit	Google
/

[aem-code-sync]

Page	Score	PSI Audit	Google
/

[aem-code-sync]

Page	Score	PSI Audit	Google
/

[aem-code-sync]

Page	Score	PSI Audit	Google
/

[aem-code-sync]

Page	Score	PSI Audit	Google
/

[tripodsan]

I would use some versioning on the lib-helix.js otherwise we get a huge mess.

at some point, it would be better to distribute this as npm...otherwise the machinery to build automated PRs against the customers project is huge. I'd rather delegate this to renovate :-)

[icaraps]

it would be better to distribute this as npm.

What would then be the recommended approach to include the npm library in a hlx project ? Import via eg https://www.jsdelivr.com/ ? Or via tooling + locally installed node_modules ? Or would hlx provide its own solution for importing the library from npm ?

[rofe]

I would use some versioning on the lib-helix.js otherwise we get a huge mess.

I'm wondering what that mess would look like. And which solution would lead to a greater mess (read: upgrade effort)...

For example, how would we handle the imports in all the block JS files? We would have to constantly update those in the boilerplate and block collection, and customers would need to do the same in their blocks whenever the lib version is updated. I don't suppose renovate can help here, can it?

[rofe]

What would then be the recommended approach to include the npm library in a hlx project ?

Importing client-side from a diferent host is a no-go for performance reasons (the mobile lighthouse score only allows for a single TLS handshake). So the only viable option would to add it to the package.json where renovate or the customer can update it. But that would mean introducing a build step (to copy the lib to the /scripts folder, which would also need to stay configurable) which is currently not required.

[mhaack]

it would be better to distribute this as npm.

and

Importing client-side from a different host is a no-go for performance reasons

It also could be hosted on a central CDN, even multipe versions and made available to the project via an internal redirect. Calls will be to customers domain, but it is provied from a central place.

[icaraps]

Importing client-side from a diferent host is a no-go for performance reasons

It would be magically awsome if hlx would auto-detect npm imports in the project and make them available on the CDN eg

import someModule from 'https://npm.hlx.live/package/version/module.js';

[dylandepass]

I will preface my comments by saying that this is draft PR and that I may not be fully in the loop on what David planned for the PR. So please disregard if my comments are a little early ;)

at some point, it would be better to distribute this as npm..

I would second this for the following reasons

1. Simplified versioning. We can make the assumption that clients will never want or need to update helix-lib but in reality this is not an absolute. The product could change, some new feature of browsers/javascript could come out we want to adopt etc. NPM gives us a clear path to upgradability.

2. Testing. This still has a problem with helix-lib tests living in boilerplate. If we are expecting clients to manually upgrade helix-lib are we also expecting them to copy over the new tests to go along with the upgrade? We also can introduce richer testing (like visual regression testing for example) if we are not concerned with bloating boilerplate.

3. I still feel like it would be nice to breakup helix-lib even more to separate concerns.

What would then be the recommended approach to include the npm library in a hlx project

I think option 2 you outlined is the right approach as 3rd party cloud bundlers will have performance issues from what I have seen. So like you said something around hlx provide its own solution for importing the library from npm.

For example, how would we handle the imports in all the block JS files?

If we keep the name of the helix-lib the same and upgrades happen to the same file in /src then no changes are required.

But that would mean introducing a build step (to copy the lib to the /scripts folder, which would also need to stay configurable) which is currently not required.

I think we can cover this with the franklin cli and not have to introduce a build step or bundler.

It also could be hosted on a central CDN, even multiple versions and made available to the project via an internal redirect. Calls will be to customers domain, but it is provided from a central place.

This could be interesting if it was something delivered by helix and we can figure out how to deliver it via same origin. One problem with this approach though is the developer experience. You lose all code hinting/completion and you can't right click on a function if you wanted to view the source.

In general I still feel like what is in scripts.js in this PR is still too much and could be made simpler for developers. What if as a product team we want to introduce or change something about the loadEager or loadLazy phase? What the builder approach I proposed gave us was more control over the decoration process while still making it flexible for the customer. I get the vibe that it may not have been well received, which is fine and I'm happy to drop the idea, but I would be interested in helping come up with something that still meets the fine balance of simple, controlled and flexible.

[aem-code-sync]

Page	Score	PSI Audit	Google
/

[aem-code-sync]

Page	Score	PSI Audit	Google
/

[aem-code-sync]

Page	Score	PSI Audit	Google
/

[aem-code-sync]

Page	Score	PSI Audit	Google
/

[aem-code-sync]

Page	Score	PSI Audit	Google
/

[aem-code-sync]

Page	Score	PSI Audit	Google
/

[aem-code-sync]

Page	Score	PSI Audit	Google
/

[davidnuescheler]

thanks to @tripodsan @rofe @dylandepass for their detailed review and thoughtful comments. i really appreciate the rigor and attention to detail. i guess that leaves us with the naming quandary...

[aem-code-sync]

Page	Score	PSI Audit	Google
/

[aem-code-sync]

Page	Score	PSI Audit	Google
/

[aem-code-sync]

Page	Score	PSI Audit	Google
/

[aem-code-sync]

Page	Score	PSI Audit	Google
/

[aem-code-sync]

Page	Score	PSI Audit	Google
/

[aem-code-sync]

Page	Score	PSI Audit	Google
/

[davidnuescheler]

thanks @kptdobe for splitting the tests... exciting.

[kptdobe]

I initially wanted to write more tests as part of this PR but I changed my mind. We should first do the split and if it gets merged, add more test (if we decide to not do the split and tests are in the PR, it is the mess).

[trieloff]

Regarding our delivery options: we have been delivering NPM libraries from rum.hlx.page:

https://github.com/adobe/helix-project-boilerplate/blob/246ea031e4c92b6610fb04274888613d73711c9f/scripts/scripts.js#L64

and

https://github.com/adobe/helix-rum-enhancer/blob/7ef6f18b3e20bf5af9fbf946628be42c4b7f52f8/src/index.js#L65

so the machinery to deliver is already there and would require only minimal tweaking of the allow-list:

https://github.com/adobe/helix-rum-collector/blob/d8bdbfb8e25281c785bfe85069dcb581a14e7df1/src/index.js#L52-L57

The one remaining thing would be to include the .hlx route in *.hlx.page so that the current third party request gets turned into a first party request.

[aem-code-sync]

Page	Score	PSI Audit	Google
/