v19.0.0 build error with Hugo

[XhmikosR]

Describe the bug
v19.0.0 build error with Hugo

To Reproduce
Steps to reproduce the behavior:

1. git clone https://github.com/twbs/blog.git -b xmr/deps
2. npm i && npm start
3. See the build error

C:\Users\xmr\Desktop\bootstrap\blog>npm start

> bootstrap-blog@1.0.0 start
> npm run serve


> bootstrap-blog@1.0.0 serve
> hugo server --port 4000 --disableFastRender --noHTTPCache --renderToMemory --printPathWarnings --printUnusedTemplates

Watching for changes in C:\Users\xmr\Desktop\bootstrap\blog\{node_modules,package.json,src}
Watching for config changes in C:\Users\xmr\Desktop\bootstrap\blog\hugo.yml
Start building sites …
hugo v0.124.1-db083b05f16c945fec04f745f0ca8640560cf1ec+extended windows/amd64 BuildDate=2024-03-20T11:40:10Z VendorInfo=gohugoio

Built in 1383 ms
Error: error building site: JSBUILD: failed to transform "/assets/js/lazyload.js" (text/javascript): No matching export in "node_modules/vanilla-lazyload/dist/lazyload.min.js" for import "default"

Expected behavior
Should not produce errors.

LazyLoad version
Please report which version of LazyLoad you're using.

• Version 19.0.0

Going back to v18.0.0 works; you can confirm this with git checkout main && npm i && npm start

[verlok]

Hey @XhmikosR,
thank you for opening this.
Yeah, we've changed the way LazyLoad is exported, I'm sorry that have impacted your build system.
You can rollback safely to 18.0.0 for now, as we try and solve this issue.
@erikyo would you have time to check this issue, which was caused by #607?

[erikyo]

Turns out that the umd export has to be set as:

exports: "default",

ref. https://rollupjs.org/configuration-options/#output-exports

@verlok can you kindly assign this one to me?

[verlok]

@erikyo assigned

[verlok]

Hey @XhmikosR,
I've just released 19.0.1 with the fix from @erikyo that should fix the problem.
Could you please try that and let me know?
Thanks

[verlok]

Hey @XhmikosR,
I’ve just tried cloning and running your repo as you instructed in your first message, and I'm glad to see the issue is solved!

If you’re happy with our support, the documentation and the effort we've been putting on this project in the latest years, I hope you might want to buy us a coffee to show your appreciation. Or sponsor us, if you're a fan.

Open-source software is great for everyone, but it takes passion and time (and coffee!) to grow and evolve.

Thank you for thinking about it.
Have a wonderful day!

[XhmikosR]

Confirmed, thanks!

Hey @XhmikosR, I’ve just tried cloning and running your repo as you instructed in your first message, and I'm glad to see the issue is solved!

If you’re happy with our support, the documentation and the effort we've been putting on this project in the latest years, I hope you might want to buy us a coffee to show your appreciation. Or sponsor us, if you're a fan.

Open-source software is great for everyone, but it takes passion and time (and coffee!) to grow and evolve.

Thank you for thinking about it. Have a wonderful day!

I feel you, I'm on the same boat myself, but OSS just doesn't pay enough, unfortunately.

Thanks again!

[erikyo]

Thank you for the report @XhmikosR! 🤗

And take a look at the new ESM version, can shrink a bit the bundle size and, in general, should perform better than the umd module!

[XhmikosR]

@erikyo unfortunately, I couldn't make Hugo which is using esbuild to use the ESM version even with format: esm in scripts.html.

I'll try to experiment more with ESM later.

[erikyo]

I was able to get it working as a esm module using the module path and replacing this line in this way:

import LazyLoad from 'vanilla-lazyload/dist/esm/lazyload'

Anyhow i'm not sure to have set correctly the esbuild here, is that correct?

{{- $esbuildOptions := dict "target" "es2019" "format" "esm" -}}

if yes it seems the esm path isn't picked correctly, maybe due the "browser" field of the package.json. Thanks for the feedback again, I'll talk to @verlok and we'll see what we can do to improve (if possible).

[XhmikosR]

Yeah, that's the correct solution AFAICT, see https://github.com/twbs/blog/tree/xmr/lazyload-esm.

I would expect esbuild to pick up the ESM build automatically when type: module is set or when an .mjs is used but none worked for me.

[XhmikosR]

Note that on the main repo docs, other packages are correctly picked up as ESM.

git clone https://github.com/twbs/bootstrap.git
npm ci
npm run docs-serve

Then check out search.js or stackblitz.js

[XhmikosR]

OK I think I found the cause. Unsure if this is considered a bug in Hugo/esbuild.

If you change your package.json properties to be relative then it works:

  "main": "./dist/lazyload.min.js",
  "module": "./dist/esm/lazyload.js",
  "browser": "./dist/lazyload.min.js",
  "typings": "./typings/lazyload.d.ts",

EDIT: I was testing the wrong branch, it doesn't fix the issue.

[erikyo]

@XhmikosR but you aren't on the wrong path... the problem is exactly that, we have to look at why Hugo is not picking the esm module (perhaps the main file is not called index and I cannot target build/esm/? file extension? ...)

[XhmikosR]

AFAICT the other two packages we use in the main repo don't use index either (only docsearch does). But StackBlitz correctly uses ESM. Weird issue for sure 🤔

…

On Sat, Mar 30, 2024, 09:28 Erik Golinelli ***@***.***> wrote: @XhmikosR <https://github.com/XhmikosR> but you aren't on the wrong path... the problem is exactly that, we have to look at why Hugo is not picking the esm module (perhaps the main file is not called index and I cannot target build/esm/? file extension? ...) — Reply to this email directly, view it on GitHub <#609 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACVLNOY35YOH64GGMLPI7DY2ZLRHAVCNFSM6AAAAABFOMFOY2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRXHE2TEOBWG4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[erikyo]

I would expect esbuild to pick up the ESM build automatically when type: module is set or when an .mjs is used but none worked for me.

That is the changes that we reverted due this issue,
#610

it is however true that Node allows you to export it in various ways, but often not all of them are supported by bundlers so we will have to find the right balance without breaking compatibility with the past and who is using this module.

[erikyo]

Apologies, but I observed the "native lazyload" version and became curious to explore its variances with various bundling methods, both with and without the module. In my testing approach, due to the continual data requests from the YouTube iframe, I rely on the initial 'finish' time recorded in my network panel within the developer tools. All tests are conducted against the localhost version, which retrieves videos from YouTube on the following page: http://localhost:4000/videos/

The "native lazyload"

Using the esm version of vanilla lazyload (and maybe some others changes)

using the umd version of vanilla lazyload

It seems that the esm version is a little faster (as promised) but nothing exceptional of course.. 😇

[erikyo]

I tried and it seems related to the "browser" field of the vanilla-lazyload package. Correctly the Hugo bundler is picking the version for the "browser" as suggested in the package.json, so in order to use the vanilla lazy load "natively" as ESM module even for the browser @verlok should update the "browser" field using the esm path. Not sure about the side-effects so this is only a recap of my findings.

If the repo is left as it is nothing bad happens, the module can be forced specifying using the path of the esm version (e.g. import LazyLoad from 'vanilla-lazyload/dist/esm/lazyload') as we did.

Mystery solved, if you can confirm @XhmikosR, just need to go temporarily to update (or remove) the path in the vanilla-lazyload package.json browser field and you will see (without any further change) that the esm module is loaded as we wanted.

@verlok please keep the module in that way for some time until the ESM support also on the node side is 100% ready.

[verlok]

I just checked the documentation for the package.json and found out that the module field is not there anymore (whilst I have one in my package.json), and it seems it got replaced from the main itself, as per:

The main field is a module ID that is the primary entry point to your program. That is, if your package is named foo, and a user installs it, and then does require("foo"), then your main module's exports object will be returned.

The documentation for the browser field says, instead:

If your module is meant to be used client-side the browser field should be used instead of the main field. This is helpful to hint users that it might rely on primitives that aren't available in Node.js modules. (e.g. window)

The problem is this module is only meant to be used in the browser, so I'm wondering why do I need to have both the browser and the main field populated, instead of only the main.

Thoughts?

[verlok]

I also asked Google Gemini, with this prompt

I'm creating a library useful to lazyload images on the web browser, and I wonder how I should value the main field and the browser field of my package.json. Do I need to value both main and browser, or just the main? In the dist folder I have both the bundled, minified version of the script, which is kinda legacy, and the entry point for importing it as an ES module.

And the following was its reply:

In your case, you'll want to define both main and browser fields in your package.json for wider compatibility. Here's a breakdown of their purposes and why you'll need both for your lazy loading library:

main field:

• This is the default entry point for your library when it's required by other Node.js modules using require().
• In your case, since you have a bundled and minified version for legacy support, this field should point to the bundled script in the dist folder (e.g., dist/lazyload.min.js).

browser field:

• This field is specifically for browsers that use a module bundler like Webpack or Rollup. It tells the bundler where to find the entry point for your library when it's imported as a module.
• Here, you'll want to point this field to the ES module version of your library in the dist folder (e.g., dist/lazyload.esm.js).

By defining both fields, you cater to different usage scenarios:

• Node.js usage: Consumers using require in Node.js will get the bundled and minified version from the main field.
• Browser usage with bundlers: When bundled for the browser using tools like Webpack, the browser field will guide the bundler to the ES module version for proper import.

Here's an example structure for your dist folder and the corresponding package.json fields:

dist/
  lazyload.min.js (bundled and minified version)
  lazyload.esm.js (entry point for ES module import)

package.json
{
  "main": "dist/lazyload.min.js",
  "browser": "dist/lazyload.esm.js",
  "module": "dist/lazyload.esm.js"  // Optional: can be an alias for "browser" for some bundlers
  // Other properties...
}

This way, your library offers flexibility for both Node.js and browser environments with bundlers.

[erikyo]

yep because that fields were "node related" check here: https://nodejs.org/api/packages.html#conditional-exports

the browser field is a "community condition definition" https://nodejs.org/api/packages.html#community-conditions-definitions and were used to override the "conditional exports"

[verlok]

To be honest I can't understand much from the nodejs.org documentation :D :D

[verlok]

But from my understanding of the npm documentation and from Gemini's answer, I would change both module and browser fields to these values

  "module": "dist/esm/lazyload.js",
  "browser": "dist/esm/lazyload.js",

What do you think @XhmikosR, @erikyo?

[erikyo]

About the "browser" field it's a way to recommend to the bundler which version to use, and of those you actually shipping all 3 would be valid for the browser (iife and esm are the best candidates)

Gemini is telling to serve the esm version as default but my suggestion is to avoid that at the moment to avoid sideeffects like the one that happened to @XhmikosR

the times are changing but we are not 100% yet :)

[erikyo]

At present, you have the option to include a note in the readme specifying that if a user intends to utilize the ECMAScript Module (ESM) version for a bundler, they should reference this specific path instead of the conventional one:
import LazyLoad from 'vanilla-lazyload/dist/esm/lazyload.js'

[verlok]

Did that here some days ago

[erikyo]

Yes I think then that it is not necessary to specify anything else... if someone really needs to, I think they will find this issue, which also explains the reason for the current situation and exactly how to solve it.

[XhmikosR]

Agreed, better keep things as is for now. Maybe mention the ESM explicit import in README.md but other than that I guess what we have now is a good compromise.