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

Provide system to force reload of files #135

Closed
amatiasq opened this issue Sep 23, 2021 · 9 comments
Closed

Provide system to force reload of files #135

amatiasq opened this issue Sep 23, 2021 · 9 comments

Comments

@amatiasq
Copy link

Following #134, we need a way to force lume to reload a file.

Example use case is if we know a.ts depends on b.ts whenever we receive a beforeUpdate event because b.ts has changed we want Lume to also re-evaluate a.ts.

We can add a.ts to event.files but that won't trigger a re-evaluation because of Source internal cache.

@amatiasq amatiasq changed the title Provide system to add files to update event Provide system to force reload of files Sep 23, 2021
@oscarotero
Copy link
Member

oscarotero commented Sep 24, 2021

Hi, @amatiasq
Thanks for keep pushing this. After thinking about it for a while, I see different things that can be solved separately.

Cached files

As you know, Lume has an internal caching system in the Source class. This is useful for pages that have the same dependencies. For example, several pages that use the same layout. Any file loaded with source.load() is automatically cached and loaded only once (this includes all page files, data files, layouts, etc).

This internal cache manages automatically file changes. So we could use the same internal cache system for modules.

At this moment, all modules are loaded using a import(), so they are cached by the module system of javascript and there's no way to reload a module. The only way to do it is by loading the javascript files as text files and evaluate them, as you did in the previous PR. This has some limitations but allows to live-reload any changed module.

My proposal is load the files and all dependencies and use the existing cache file to store that. To make it, we can change the signature of the loaders, addding a second argument with the Source instance. This allows to load all dependencies and save the content in the cache:

import textLoader from "lume/core/loaders/text.ts";

// Fake code, for illustration purposes:
export default async function loader(path: string, sources: Source): Data {
    const content = await textLoader(path);

    // Get all dependencies of this file
    const depsContent = getDependencies(content, async (dependencyPath) => {
            // We load the dependencies with the Source instance
            // so they are automatically cached and downloaded only once
            return await sources.load(dependencyPath, textLoader);
    });

    return compileAndRun(content, depsContent);
}

The idea is that one loader can invoke other loaders using the source instance, to load other dependencies that could be cached.

This means that if a.ts imports b.ts and it imports c.ts, loading a.ts, we would load and save the content of the three files in the cache . If one of these files change, the cache would remove the old cached content automatically of this specific file.

Dependencies

I think your approach of save the dependencies in the Source file, in a private dependants property is good. The question is, how to store these dependencies? An idea would be change the Sources.load() signature to include a third argument to define a file as a dependant of other. For example:

import textLoader from "lume/core/loaders/text.ts";

// Load a file using a textLoader
sources.load("a.ts", textLoader);

// Load a dependency of a.ts
sources.load("b.ts", textLoader, "a.ts");

// Load another dependency of a.ts
sources.load("c.ts", textLoader, "a.ts");


// Load the same file again (it's cached, so won't be loaded)
// But has a different dependant, so it will be saved
sources.load("c.ts", textLoader, "d.ts");

This would save, automatically the dependencies relations, and we know that "b.ts" and "c.ts" are dependencies of "a.ts" and "c.ts" is a dependency of "a.ts" and "d.ts".

Note: we don't need to save the exact dependency tree. Just the relation of the main entry point (the initial file loaded) with the dependencies, no matter if they are direct dependencies or sub-dependencies.

So, in the previous example, we only have to include the main dependent path for all dependencies:

import textLoader from "lume/core/loaders/text.ts";

// Fake code, for illustration purposes:
export default async function loader(path: string, sources: Source): Data {
    const content = await textLoader(path);

    // Get all dependencies of this file
    const depsContent = getDependencies(content, async (dependencyPath) => {
            // We load the dependencies with the Source instance
            // so they are automatically cached and downloaded only once
            // WARNING: The third argument indicates that this file is a dependency of the main entry point (path)
            return await sources.load(dependencyPath, textLoader, path);
    });

    return compileAndRun(content, depsContent);
}

Watch mode

The function to reload a file in watch mode is this https://github.com/lumeland/lume/blob/master/core/site.ts#L265 and this function calls the loadFile function of Source (https://github.com/lumeland/lume/blob/master/core/source.ts#L92).

So, the idea is that calling loadFile("c.ts"), lume knows that this is a dependency of two main entries ("a.ts" and "d.ts", so it will reload these pages, even if they didn't change).

Thoughts?

@amatiasq
Copy link
Author

amatiasq commented Sep 24, 2021 via email

@oscarotero
Copy link
Member

Oh i see, you're proposing to split the Readding From Disk from execution
of the files.

Yes, exactly. The same dependency can be used several times, but we only read it once.

We can use url parsing to evaluate the module's code but without the
sourcemaps we won't have the list of dependencies of the files. We could
use that rust library you mentioned before.

I think it's better because the rust library only return the graph of dependencies (is the equivalent to run the comand deno info a.ts), but don't execute anything, so it's faster that bundle, create a source map, parse it and inspect.

I was also considering we could check if the loaded file is outside 'src'
(but inside 'cwd()'?) and watch them too.

This can't be done. The watcher only see changes in the src directory. The idea is that all src files needed to build the site are in this directory. The cwd can change (you can build the same site from different cwd). Any external file (outside of this directory or a remote dependency) won't be watched.

@ejsmith
Copy link
Contributor

ejsmith commented Sep 24, 2021

I am just jumping into the middle of this and don't know anything, but I'm curious if doing the source maps helps with debugging? Ideally, it would be nice when having trouble to be able to step through code even in templates. I know that nunjucks is supposed to be adding source maps support at some point.

@amatiasq
Copy link
Author

amatiasq commented Sep 24, 2021 via email

@oscarotero
Copy link
Member

oscarotero commented Sep 26, 2021

@amatiasq I just created a branch implementing a new way to watch files, compatible with javascript modules:

  • The build and watch are executed in a Worker code here.
  • If at least one of the changed file is javascript or typescript, the worker is terminated, and a new one is created, so the entire site is built again with fresh content.

This system has two drawbacks:

  • On change a js/ts file, the entire site is rebuilt, so it may be slower than just update the changed file.
  • There's no support for import maps in Workers see issue so the _config.ts file cannot use bare imports.

I've added this mode under the --rebuild flag, so both modes are available. lume --serve will work as now, and lume --serve --rebuild enables this watch mode.

what do you think?

@amatiasq
Copy link
Author

@oscarotero running the code in a Worker is a great improvement regarding modules isolation!

I'll need time to carefully read and understand the new code so I can understand why those limitations apply, sadly I doubt I can get half as much time as last week.

@oscarotero
Copy link
Member

Don't worry, no rush :)
Thanks!

@oscarotero
Copy link
Member

The new version 1.1.1 was released. It includes the new experimental watcher (enabled with lume --serve --experimental or lume --watch --experimental. Once Deno fixes some issues, it could replace in the future the default watcher.

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

3 participants