remy / inliner

Node utility to inline images, CSS and JavaScript for a web page - useful for mobile sites
MIT License
1.1k stars 165 forks source link

Add an option to specify the "root" directory #171

Open Fuco1 opened 6 years ago

Fuco1 commented 6 years ago

Resolution of relative paths does not seem to work properly, I have to run the node process (or change with process.chdir) in the directory where the template is and the paths are resolved properly by node relative to it.

It would be nice to be able to specify the root directory so that I can run node in different directory and have assets and template in a different one.

This should probably not "just call chdir" becuase that affects all the other code running in the same process as well which might not be wanted.

remy commented 6 years ago

Can I assume that you're using this as a required module and not from the command line?

Fuco1 commented 6 years ago

@remy Correct.

Right now I use this wrapper

const path = require('path')
const Inliner = require('inliner');

export default function (html: string, templatePath: string) {
    return new Promise(function (resolve, reject) {
        process.chdir(path.dirname(templatePath))

        new Inliner({source: html}, function (error, res) {
            if (error !== null) {
                return reject(error)
            }
            return resolve(res)
        })
    })
}

Which is then used as such

import inline from './inline'

const packedHtml: Uint8Array = await inline(html, templatePath)

I'm sending in the html which is a string I generate in the program. I pass the templatePath only to be able to do the chdir (template is a handlebars template I resolve in the program then pass the resulting html to the Inliner)

Maybe there's a better way to do this and avoid the trouble, I wasn't able to find one.

remy commented 6 years ago

No, it's fine, it's more that the chdir is important for the command line use, but I can see how you wouldn't want it doing that if required. Do you want to take a shot a PR + test?

Fuco1 commented 6 years ago

@remy Actually this is not a solution. Because while the promise is "being resolved" another thing can get executed (say another inline request) and it can change the directory before actual inliner code will run then it will have wrong context.

Or am I misunderstanding your request? I don't know where and how the paths would need to be modified to take into account some "root" directory.

remy commented 6 years ago

How about if you can script a simple test that replicates the problem, and I'll take a look. I think the chdir is only in a single place, and I'm pretty sure it's just so that files can be resolved when working off the CLI. So if I have something when inliner is required, then I think it'll be relatively easy to spot if the change would work.