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

Support chunk lazy loading #15

Closed
brenogazzola opened this issue Sep 9, 2021 · 29 comments
Closed

Support chunk lazy loading #15

brenogazzola opened this issue Sep 9, 2021 · 29 comments

Comments

@brenogazzola
Copy link
Contributor

brenogazzola commented Sep 9, 2021

Our production app using Webpacker 6 currently has many files that employ lazy loading of chunks:

// Only load Trix when necessary
  async bootstrap () {
    const { default: Trix } = await import(/* webpackChunkName: "trix" */ '../../chunks/trix')
    return Trix
  }

We are switching to jsbundling, but the code above is broken. It seems that Webpack can't find the chunk because of the fingerprint that Sprockets add to the end of the file name.

Is this possible with jsbundling right now, or would it be necessary for me to open a PR to add the support? I tried reading through the Webpacker code, but couldn't figure out what it was doing to make this work.

@dhh
Copy link
Member

dhh commented Sep 10, 2021

There's not a straight path I can see for relying on Webpack's internal chunking with this approach. You'll have to split out the lazy-loaded elements into their own entry points. That's a bit of a trade-off with this model.

@dhh dhh closed this as completed Sep 10, 2021
@brenogazzola
Copy link
Contributor Author

brenogazzola commented Sep 10, 2021

There's an alternative: Disable Sprockets fingerprinting and rely on the bundlers' content hash. Webpack and Esbuild have them, but I'm not sure about Rollup.

I don't think we currently have way to tell Sprockets to skip the digest of specific file types, but I think we could add a skip_digest_for configuration option that takes an array of file extensions. Then we can change the stats_digest method in the PathDigestUtils file to check that configuration:

def stat_digest(path, stat)
  if stat.directory?
    # If its a directive, digest the list of filenames
    digest_class.digest(self.entries(path).join(','.freeze))
  elsif stat.file?
    # If its a file, digest the contents
    digest_class.file(path.to_s).digest unless Sprockets.skip_digest_for.include?(File.extname(path))
  else
    raise TypeError, "stat was not a directory or file: #{stat.ftype}"
  end
end

With this the files will still have a different name if they changed, keeping the expected functionality, Sprockets will still add them to its manifest file, but any bundler feature that relies on the file name (like lazy loading chunks) will not break.

@dhh
Copy link
Member

dhh commented Sep 10, 2021

Please do investigate that! I like the idea that Sprockets could even auto-detect a pattern, and just skip files that have already been digested.

@brenogazzola
Copy link
Contributor Author

I considered auto detection, but thought it might be considered too brittle.
I will take a stab at this and submit a PR to sprockets when I get something working.

@dhh
Copy link
Member

dhh commented Sep 10, 2021

I'd say we'd want to just auto-detect that the digest format matches what sprockets is already generating. And then we'll recommend/configure webpack to produce in the same format (same length/charset).

@brenogazzola
Copy link
Contributor Author

Here it goes: rails/sprockets#714.
Took me half a dozen tries to figure out where to plug the behavior without breaking something else 😅

@dhh dhh reopened this Sep 11, 2021
@dhh
Copy link
Member

dhh commented Sep 11, 2021

What's needed for the default webpack config to produce chunks with hashes in them? Also, I vaguely remember that if you digest hashes, there's an issue with the webpack dev server?

@brenogazzola
Copy link
Contributor Author

brenogazzola commented Sep 11, 2021

This should config should match what sprockets currently does:

output: {
    path: path.resolve(__dirname, 'app/assets/builds'),
    filename: '[name]-[contenthash].js',
    chunkFilename: '[name]-[contenthash].chunk.js',
    hashFunction: 'sha256',
    hashDigestLength: 64
},

The current version of sprockets, assets:precompile generates these double digested files:

public/assets/fudgeballs-3e05d90ab38c43e41a3f1400bbae046de14b44df67edf0506fa0d68d744eaf20-9c3018226817bce637b2a39a98e52a3c818cd7e0ed397ed530353373919744e5.js
public/assets/fudgeballs_active_storage-8ff22166fa4c88fb20d8dfe95327a62decbd85690571a8ed9bffa261eb30ac8c.chunk-86b6f708a1df94cf6bf29f7177b184f6a86037d4bf396a0659d0dbc6b2a84931.js
public/assets/fudgeballs_clipboard-8ff22166fa4c88fb20d8dfe95327a62decbd85690571a8ed9bffa261eb30ac8c.chunk-701271f31e3fb24353d1e9e50a24d127f0391befcf5e13fe3074467e6304befa.js

The PR version correctly skips the fingerprinting:

public/assets/fudgeballs-3e05d90ab38c43e41a3f1400bbae046de14b44df67edf0506fa0d68d744eaf20.js
public/assets/fudgeballs_active_storage-ba5fc1e55a50a4647ee6e65f8270a22ddec5cf72618fbd5c71088cb0f2b9065d.chunk.js
public/assets/fudgeballs_clipboard-8ff22166fa4c88fb20d8dfe95327a62decbd85690571a8ed9bffa261eb30ac8c.chunk.js

There is however one configuration that Webpacker 5/6 has that I'm still missing and will try to find. This is the runtime.js that it produces, in order to figure out the correct path to the chunks:

n.u = e=>"js/" + {
  712: "fudgeballs_active_storage",
  875: "fudgeballs_clipboard",
  998: "trix"
}[e] + "-" + {
  712: "de39e19b81af6079c9306c79ae331ebc",
  717: "9957ff62fb2b6e6cbc110ad795b009f4",
  998: "4afa271d098ab9c491abfbf4838c7bf2"
}[e] + ".chunk.js",

This is what cssbundling with the config above is producing:

i.u = t=>({
  712: "fudgeballs_active_storage",
  717: "fudgeballs_invitations",
  875: "fudgeballs_clipboard"
}[t] + ".js")

So I've figured out how to produce sprockets like digests, but I still need to figure out how to tell Webpack to properly compile the js code with the digest.

@brenogazzola
Copy link
Contributor Author

I'm not sure what you mean by 'digest hashes', but Webpacker 5/6 are working out of the box, no extra configs needed and the dev-server fine. We've been in this setup for months and both our local and production environments have to trouble finding the chunks.

The cssbundling gem has no problems in local environment using yarn build --watch. And production is what we are trying to fix.

@dhh
Copy link
Member

dhh commented Sep 11, 2021

Ah. Sweet. So you’re all good with your fork if sprockets with the patch?

@brenogazzola
Copy link
Contributor Author

Yup, it's all working now: #21 👍

@tleish
Copy link

tleish commented Sep 12, 2021

We also use lazy loading through webpack, especially with stimulusjs controllers which rely on large js libraries. We tested jsbundling with esbuild. Loved the speed of esbuild over webpack, but couldn't get lazy loading modules to work because esbuild only supports "splitting" and lazy loading with ESM output. We can't use ESM modules because importmap-rails requires Rails 7 in order to use ActionCable.

@dhh
Copy link
Member

dhh commented Sep 12, 2021

Will have some alpha releases of those Rails JS modules that include ESM shortly. Hoping next week 👍

@theodorton
Copy link

theodorton commented Sep 12, 2021

Would love to see something similar implemented for esbuild. I'm struggling to get chunks to work correctly and running into all kinds of weird precompilation issues on Heroku as a result of my workaround.

Unfortunately esbuild uses Base32 for digests:
https://github.com/evanw/esbuild/blob/e79ea6f4cc777d82e968fd644e1bec4efe3b7578/internal/bundler/bundler.go#L933-L935

So it would either require an upstream patch to esbuild to support md5 or some other hash function (which I would find unlikely given the highly opinionated approach), or adding support for Base32 digests in sprockets. Biggest downside of the latter would be that Base32 is not supported by ruby's standard library and might require adding an optional dependency on the base32 gem.

@theodorton
Copy link

Here are the changes required in esbuild to use SHA256 for digests:
evanw/esbuild@master...onelittle:sha256-hashing

Not sure what's the best approach for getting this merged upstream, but it makes esbuild --splitting work out of the box with the asset pipeline with @brenogazzola's changes to sprockets.

Binaries published on npm for testing:
https://www.npmjs.com/package/@onelittle/esbuild-linux-64
https://www.npmjs.com/package/@onelittle/esbuild-darwin-64

@dhh
Copy link
Member

dhh commented Nov 11, 2021

With esbuild, can't we lean on the same .digested marker? Why do we need to know the digest format?

@brenogazzola
Copy link
Contributor Author

@theodorton How are you handling the entrypoint? On my tests with esbuild I could not make it generate chunks with digest and the entrypoints without the digest. If the entrypoint has a digest, the asset helper cannot find in the manifest.

@DTrejo
Copy link

DTrejo commented Nov 12, 2021

(thank you for jsbundling and rails, really enjoying the speed compared to webpack!)

Here is one solution that works for esbuild + preact + esm modules in the browser:

Note the --splitting and --format=esm flags

esbuild app/javascript/*.* --bundle --outdir=app/assets/builds --loader:.js=jsx --jsx-factory=h --jsx-fragment=Fragment --splitting --format=esm"

Add to gemfile, this leaves the chunks untouched by creating a non-digest copy, and served by rails, for use by dynamic imports

gem "non-stupid-digest-assets" 

@navidemad
Copy link
Contributor

I'm not sure if the usage of the gem "non-stupid-digest-assets" is still needed now @DTrejo ?

@thanosbellos
Copy link

I got splitting working for esbuild by using the following configuration in my esbuild.config.js file

const watch = process.argv.includes('--watch') && {
  onRebuild (error) {
    if (error) console.error('[watch] build failed', error)
    else console.log('[watch] build finished')
  }
}

require('esbuild').build({
  entryPoints: ['app/javascript/application.js', 'app/javascript/admin.js'],
  bundle: true,
  outdir: 'app/assets/builds',
  sourcemap: true,
  splitting: true,
  chunkNames: '[name]-[hash].digested',
  format: 'esm',
  watch: watch
}).catch(() => process.exit(1))

I am still on sprockets and in order for this config to work we need this pull request from @dhh. I guess a similar pull request is needed in order to make it work with propshaft.

Also if i can recall correctly, splitting requires esm output format. That means that we have to include our js files like this:

    <%= javascript_include_tag "application", "data-turbo-track": "reload", type: "module" %>

@joker-777
Copy link

Hi, I'm trying to get this working with jsbundling-webpack by adding filename: "[name]-[contenthash].digested.js" to the config. It works so far that it doesn't append a new hash to the files but the problem is now that it expects me to specify the [contenthash].digested when including a javascript file. Looking at .sprockets-manifest-<hash>.js I can see for example:

"polyfills-c3ced8e904527e202532959a7f97306113b38175efed8c99dcd22e4900671573.digested.js":{"logical_path":"polyfills-c3ced8e904527e202532959a7f97306113b38175efed8c99dcd22e4900671573    .digested.js","mtime":"2022-02-07T16:47:41+00:00","size":200358,"digest":"3e1d1bfa974736260547a1e040af0f4dcc1e5166fd96aa21b9122c7a1eb44000","integrity":"sha256-Ph0b+pdHNiYFR6HgQK8PTcweUWb9lqohuRIseh60QAA="}

but I would rather expect/want to see something like this

"polyfills.js":{"logical_path":"polyfills-c3ced8e904527e202532959a7f97306113b38175efed8c99dcd22e4900671573    .digested.js","mtime":"2022-02-07T16:47:41+00:00","size":200358,"digest":"3e1d1bfa974736260547a1e040af0f4dcc1e5166fd96aa21b9122c7a1eb44000","integrity":"sha256-Ph0b+pdHNiYFR6HgQK8PTcweUWb9lqohuRIseh60QAA="}

Has anyone made this work?

@tsrivishnu
Copy link

tsrivishnu commented Feb 7, 2022

@joker-777 I suppose you don't want filename to have [contenthash].digested. You might need the [contenthash].digested only on chunkFilename instead so that you can lay load the chunks. Of course, this depends on your setup but in general, I suppose this is how most of us use.

This is how our webpack output config is:

module.exports = (env) => ({
  mode: "production",
  target: "web",
  entry: entry("./app/javascript/packs/*.js"),
  output: {
    // filename: "[name]-[contenthash].digested.js", // asset pipeline expects the names without hashes
    chunkFilename: "[name]-[contenthash].digested.js",
    sourceMapFilename: "[file]-[fullhash].digested.map",
    path: path.resolve(__dirname, "app/assets/builds"),
    pathinfo: true,
    publicPath: "/assets/",
    clean: {
      keep: /.keep/,
    },
  },
  // ...
});

See: https://srivishnu.totakura.in/2022/01/19/overcoming-challenges-with-jsbundling-rails-with-webpack-5-on-production.html#chunks-sourcemaps-and-asset-pipeline-sprockets

@joker-777
Copy link

@tsrivishnu Thanks a lot for your response. It makes completely sense :)

@joker-777
Copy link

@tsrivishnu How do you handle images (png) which you import into your javascript files e.g. import image from "path/to/image.png"?

@joker-777
Copy link

Nevermind, assetModuleFilename: "asset-[contenthash].digested[ext]" works for me.

@michelson
Copy link

michelson commented Feb 11, 2022

in esbuild I had an issue with how the images are hashed, I've added a monkey patch for sprockets, if anyone interested

put this in: /config/initializers/sprockets_patch.rb

# remove this when https://github.com/rails/sprockets/pull/726
Sprockets::DigestUtils.module_eval do |variable|
  def already_digested?(name)
    name =~ /-([0-9a-zA-Z]{7,128})\.digested/
  end
end

for the esbuild config I've used:

publicPath: "/assets",
assetNames: '[name]-[hash].digested',
outdir: 'app/assets/builds',

@brenogazzola
Copy link
Contributor Author

This is now supported in webpack/esbuild and propshaft/sprockets.

@AlecRust
Copy link

This is now supported in webpack/esbuild and propshaft/sprockets.

Got any links to examples of this?

@sifear
Copy link

sifear commented Nov 17, 2022

@AlecRust If you are still looking for an answer, I have one for Webpack that I condensed from this thread, where I digest chunks and let sprockets to digest entrypoints.
It seems sprockets can detect filenames that are digested with shaXXX (to avoid double digesting), you just have to match the algorithm with Webpack's hashDigestLength. Can be looked up here:
https://github.com/rails/sprockets/blob/1276b431e2e4c1099dae1b3ff76adc868c863ddd/lib/sprockets/digest_utils.rb#L20-L25
For sha256 its 32, which is hashDigestLength of 64 in Webpack:

{
   ...
   entry: {
        application: "./app/javascript/application.js"
   },
   output: {
        filename: "[name].js",
        chunkFilename: "[name]-[contenthash].js",
        sourceMapFilename: "[file]-[fullhash].map",
        path: path.resolve(__dirname, "app/assets/builds"),
        hashFunction: "sha256",
        hashDigestLength: 64,
    },
    ...
}

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

No branches or pull requests