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

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

Open
Fuco1 opened this issue Feb 9, 2018 · 5 comments
Open

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

Fuco1 opened this issue Feb 9, 2018 · 5 comments

Comments

@Fuco1
Copy link

Fuco1 commented Feb 9, 2018

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
Copy link
Owner

remy commented Feb 9, 2018

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

@Fuco1
Copy link
Author

Fuco1 commented Feb 9, 2018

@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
Copy link
Owner

remy commented Feb 9, 2018

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
Copy link
Author

Fuco1 commented Feb 9, 2018

@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
Copy link
Owner

remy commented Feb 9, 2018

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.

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

2 participants