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

feat: Improved HMR and Cache management on Webpack 5 #136

Closed
wants to merge 27 commits into from

Conversation

ScriptedAlchemy
Copy link

@ScriptedAlchemy ScriptedAlchemy commented Nov 13, 2020

Better cache management within webpack
Faster HMR
Simpler resolution compared to before

@ScriptedAlchemy ScriptedAlchemy changed the title Improved HMR and Cache management on Webpack 5 feat: Improved HMR and Cache management on Webpack 5 Nov 13, 2020
});

const snapshot = Object.assign({}, config.snapshot);
const mainPkg = require(pkgUp.sync());
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for better caching, it might be worth traversing over the transpiled modules package.json files (the symlinked ones at least)

@@ -187,6 +188,30 @@ const withTmInitializer = (modules = [], options = {}) => {
`**node_modules/{${modules.map((mod) => `!(${mod})`).join(',')}}/**/*`,
];

if (isWebpack5) {
const checkForTranspiledModules = (currentPath) =>
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to check if its symlinked or not before excluding it from webpack cache, otherwise we are rebuilding uncached modules that are simply in ES6, but not in the monorepo as a workspace package

@ScriptedAlchemy
Copy link
Author

@martpie this improves HMR build speed alot, for your review

@martpie
Copy link
Owner

martpie commented Nov 17, 2020

Thank you very much, can you fix the pipeline? It looks like your removed some changes I've made to the README as well

@martpie martpie changed the base branch from nextjs-10 to nextjs-10-experimental-2 November 17, 2020 14:46
@martpie
Copy link
Owner

martpie commented Nov 20, 2020

A couple of additional things:

@martpie
Copy link
Owner

martpie commented Nov 20, 2020

Ok, good news, I've managed to make it work. There was an issue in my setup.

So now, Webpack 5 HMR works, but is unoptimized as you probably realized when tweaking managedPaths. I will try to see if there is a way of making managedPaths to still include node_modules, but ignore the path of the resolved modules.

@martpie
Copy link
Owner

martpie commented Nov 20, 2020

-> webpack/webpack#12036

@ScriptedAlchemy
Copy link
Author

@martpie im going to copy paste an update for you. Right now this works for my company and is monorepo heavy so you might want to take some inspiration from it, but it might not work outside yarn workspaces, likely because of the set of package resolutions i do upfront.

Other improvements, I've got ESM and CJS resolving and webpack HMR cache seems to be acceptable

@ScriptedAlchemy
Copy link
Author

Highlights here

This solves for packages like geodesy where there's no field to resolve to and its an ESM module, i can't use import.meta so if I'm not able to resolve it, i try resolving its package.json to grab the path.

/**
 * Resolve modules to their real paths
 * @param {string[]} modules
 */
const generateResolvedModules = (modules) => {
  const resolvedModules = modules
    .map((module) => {
      let resolved;

      try {
        resolved = resolve(__dirname, module);
      } catch (e) {
        try {
          resolved = resolve(__dirname, path.join(module, 'package.json'));
        } catch (fallbackError) {
          console.error(e);
          console.error(fallbackError);
        }
      }

      if (!resolved)
        throw new Error(
          `next-transpile-modules: could not resolve module "${module}". Are you sure the name of the module you are trying to transpile is correct?`
        );

      return resolved;
    })
    .map(path.dirname);

  return resolvedModules;
};

Match and unmatch are not reliable enough with the current resolutions path. For example i have main:src/index.js but i also have /pages as a sibling to /src

dirname transpiles everything in src, but not pages. So i need to go up and resolve to the root package then match an unmatch against its module root.


    // Generate Webpack condition for the passed modules
    // https://webpack.js.org/configuration/module/#ruleinclude
    const match = (request) =>
      resolvedModules.some((modulePath) => {
        const resolveRequestRoot = path.dirname(pkgUp.sync({ cwd: request }));
        if (resolveRequestRoot.includes(modulePath)) {
          return true;
        }
        const resolvePackageRoot = path.dirname(pkgUp.sync({ cwd: modulePath }));

        if (resolveRequestRoot.includes(resolvePackageRoot)) {
          return true;
        }

        return request.includes(modulePath);
      });

    const unmatch = (request) =>
      resolvedModules.every((modulePath) => {
        const resolveRequestRoot = path.dirname(pkgUp.sync({ cwd: request }));
        if (!resolveRequestRoot.includes(modulePath)) {
          return true;
        }
        const resolvePackageRoot = path.dirname(pkgUp.sync({ cwd: modulePath }));

        if (!resolveRequestRoot.includes(resolvePackageRoot)) {
          return true;
        }

        !request.includes(modulePath);
      });

Also i see you've added funding to this project, ill be signing up as a personal sponsor, this isn't easy project :)

@ScriptedAlchemy
Copy link
Author

I am still working through a this error though.

Failed to compile.

../../node_modules/@lululemon/ecom-core/src/components/Error/error.module.scss
CSS Modules cannot be imported from within node_modules.
Read more: https://err.sh/next.js/css-modules-npm
Location: ../../node_modules/@lululemon/ecom-core/src/components/Error/index.js

@martpie martpie changed the base branch from nextjs-10-experimental-2 to nextjs-10 November 20, 2020 22:14
@martpie
Copy link
Owner

martpie commented Nov 20, 2020

So let me get this straight: the strategy is to identify the location of the package that won't change (meaning, all the packages, except the ones that are transpiled), finding them (they can be at really different locations due to yarn workspaces etc) and adding them to managed paths to improve the cache performances. Right?

@ScriptedAlchemy
Copy link
Author

Yeah. Would you wanna do a zoom call, perhaps we can combine our knowledge and solution something out.

Working on my fork has been one of the more interesting challenges and how NTM actually manipulates next has been a huge unlocker for me to use similar conventions to do the same elsewhere.

Basically we only need to find paths to package.json files that will not change. Webpack, by default, has managedPaths:[/node_modules/]

If we were able to link them to another directory and have webpack resolve node_modules + some other directory it might work around manually managing cache - tho this might cause node resolution issues outside webpack

Check out the snapshot documentation on webpack docs. One problem my implementation has is that CI runs out of memory if I use the cache manager. So I can only do it in dev. The reason is because I'm setting cache to memory, and it would be good to check how to, if possible get file system to work.

With your latest work. Removing unmatch, solved all remaining issue I had with css errors.

Enhanced resolve works. But only for cjs packages. So if it's pure ESM, there's no module, main field. So I have to try, catch (like you do, but then try and resolve to package.json)

So I resolve transpiled modules, check if they are symlinked - if so remove the from the cache, then keep searching for other modules that need transpilation. Allow webpack to cache those.

So we almost have to do a lookup on all packages that are being used and manually manage them.

In all cases I don't need to pass files, just the package.json - which Webpack will know what to do with after.

Tobias initially said we should try and make resolve symlinks work. So instead of using node modules at all, it resolves to the actual location we edit in the monorepo. I tried this but ran into loader errors saying modules can't be outside x directory or whatever.

I know you're moving away from regex, but that means that work for cache management since it takes either absolute paths or regex

Right now my fork, there's a open or in it for resolve symlinks with my latest work.

I use this project heavily, we are in the middle of company wide webpack and next 10 upgrades. NTM has been getting a lot of attention as of late. But my fork has hmr working well enough that it's around as fast as webpack 4 was. But I know it's slower than it's supposed to be - scss seems to be very slow or I'm not caching some styles correctly. Also my projects are very very large - so it's hard to tell if my cache management is off or if the size of my projects are just that big.

Anyways would love to collaborate with you on a solid solution - i do have a lot of stake in this plugin.

@ScriptedAlchemy
Copy link
Author

Without managing the paths. Webpack HMR rebuilds everything attached to changed file and parent. Since next no longer uses its own cache plugin as much in WP5, we have to make Webpack cache the right stuff ourselves otherwise it's extremely slow on HMR. Try setting cache=false and you'll see haha.

So right now I'm finding dependencies (not dev deps) then resolving them. Then everything else that passes webpack goes through findPkg to get the root of that package and manage it

@martpie martpie closed this Nov 26, 2020
@martpie martpie deleted the branch martpie:nextjs-10 November 26, 2020 12:32
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

Successfully merging this pull request may close these issues.

2 participants