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

Error: "project root is outside of project" #17

Closed
alextreppass opened this issue Apr 4, 2016 · 23 comments
Closed

Error: "project root is outside of project" #17

alextreppass opened this issue Apr 4, 2016 · 23 comments

Comments

@alextreppass
Copy link
Contributor

Hi,

Due to various reasons in our project, the client builds into the 'public' folder of our rails app. For illustration purposes:

  • server/client - client dir (where webpack.config.js lives), and where the build is run from
  • server/public - webpack config.output.path dir, where webpack builds relative to

It looks like this plugin relies on workingDir (i.e. process.cwd()) over webpack's config.output.path.

Would it be ok to change this? Webpack is building happily into a folder that isn't the cwd.

What I'm looking for is to remove / clean the files pre-build, in:

  • server/public/scripts and server/public/styles
  • leave anything else in server/public intact

Thanks!

@johnagan
Copy link
Owner

johnagan commented Apr 4, 2016

@alextreppass I'm not opposed to it. Is it something you can propose in a PR?

@chrisblossom
Copy link
Collaborator

@alextreppass You do not need to change anything. process.cwd() is the safe default for the root option.

Where projectRootDir is set: https://github.com/johnagan/clean-webpack-plugin/blob/master/index.js#L41

var CleanWebpackPlugin = require('clean-webpack-plugin');

module.exports = {
  plugins: [
    new CleanWebpackPlugin(['dist', 'build'], {
      root: '/full/project/path/server/public',
      verbose: true, 
      dry: false
    })
  ]
}

@alextreppass
Copy link
Contributor Author

I'll give it a go, thanks!

@alextreppass
Copy link
Contributor Author

Hmm still running into that issue I'm afraid.

The error message I get is: clean-webpack-plugin: /path/to/server/public/ project root is outside of project. Skipping all...

Here's my config:

 new CleanWebpackPlugin(['scripts', 'styles'], {
    root: TARGET_DIR,
    verbose: true,
    dry: false
  }),
  • TARGET_DIR is something like /path/to/server/public. This is webpack's config.output.path dir.
  • process.cwd() (from where I'm running webpack) is /path/to/server/client

From within the client dir, it's a traversal of ../public to get to the webpack output dir.

I'll fork and see if I can get it working. Bear with me!

@chrisblossom
Copy link
Collaborator

@alextreppass Could you provide a failing test case? test/test.js

@chrisblossom
Copy link
Collaborator

I believe this is showing the issue:

    it('root outside of process.cwd()', function () {
      var outsideProjectDirOne = '/test/dir/_one';
      // outsideProjectDir === '/test/dir'
      cleanWebpackPlugin =
        new CleanWebpackPlugin([outsideProjectDirOne], { root: outsideProjectDir, dry: true });
      result = cleanWebpackPlugin.apply();

      expect(result[0].output).to.equal('removed');
    });

index.js#L89 is where the check is failing.
I think adding your suggestion to check config.output.path on the failing path is a good idea. Might also be a good idea to add an option to skip that check for other edge use cases.

@alextreppass
Copy link
Contributor Author

Here's a diff where I've got it working for my needs: alextreppass@a098e76

It doesn't seem that webpack exposes wider config (like output.path) to plugins, so I re-worked it to only block cleaning out folders that are outside of the root plugin configuration option.

I also had to remove the insideFailCheck validation.

I'm seeing if I can tweak the tests now. I'm adding some folders in the test setup to verify my approach

@chrisblossom
Copy link
Collaborator

I don't think removing all of the safety checks is a good solution. Try my latest branch https://github.com/chrisblossom/clean-webpack-plugin

@alextreppass
Copy link
Contributor Author

Got the tests passing in #18 - I had missed the windows_spec.js file, whoops.

I think I removed only the checks that were ensuring the root was under the project dir.

I've tried to ensured that any dirs to be deleted must be under the given 'root' dir (unrelated to the project / working dir).

@alextreppass
Copy link
Contributor Author

Nice, your approach works great, just tested it here locally. 👍

I guess the question for your consideration is:

  • do we tie it implicitly to webpack (this is probably fine, it's a webpack plugin after all!), or
  • do we change the semantics about 'projectDir' vs 'rootDir'.

I'd be happy with either :)

@chrisblossom
Copy link
Collaborator

Just checking to make sure the files being deleted is inside of root does simplify the plugin. Also it wouldn't break with any Webpack API changes.

Also, I don't think it would break anyone's current config. Thoughts? Only downside I can think of it potentially makes the plugin less "safe" from deleting files/directories by mistake.

@alextreppass
Copy link
Contributor Author

Yeah it might be more portable -- do webpack document the compiler.options in-scope? If they changed how that's exposed we'd still be fine.

I googled around a bit for compiler.options and it seems webpack-dev-server might not have an output path, so sniffing webpack config might not work for that case. [1]

Semantics - yeah I agree it'd be compatible with current config, we're still telling the plugin "under this root folder, please delete this folder" (or these folders)

I agree the potential danger is the main concern. As you say, what if the user mistakenly puts in a root value that they shouldn't?

How about a new config option like allowExternalRoot, which is false by default? They'd have to look in the README and opt-in to using it. Would that be overkill?

@chrisblossom
Copy link
Collaborator

I am happy with removing the process.cwd()/running directory check as long as absolute root paths for root is still enforced. Seems like most rounded solution to me.

@alextreppass
Copy link
Contributor Author

@johnagan - are you ok with #18 as a solution to this?

@johnagan
Copy link
Owner

@alextreppass I'd want @chrisblossom's sign-off because some of this is his code too.

@chrisblossom
Copy link
Collaborator

I created a PR@alextreppass#1 for @alextreppass to look over. Everything looks good to me outside of that.

@alextreppass
Copy link
Contributor Author

Hey thanks, I'll integrate this into #18.

Edit: that's now done, let me know if you want me to make any more changes!

@alextreppass
Copy link
Contributor Author

alextreppass commented Apr 26, 2016

@chrisblossom / @johnagan - I merged Chris' alextreppass#1 code into my branch; #18 is ready to go if you guys are happy.

Thanks!

@chrisblossom
Copy link
Collaborator

Looks good to me.

@johnagan
Copy link
Owner

Deployed v0.1.9 to npm.

Thanks everyone 👍

@alextreppass
Copy link
Contributor Author

Cheers 👍

@pmunin
Copy link

pmunin commented Sep 30, 2018

Does not for work projects with multiple workspaces, where there is some webpack.config.base.js injecting clean-webpack-plugin in it located in another folder outside the subpackage.
E.g.:
=packages
==shared
===webpack.config.base.js
==subpackage1
===webpack.config.js

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

5 participants
@johnagan @alextreppass @chrisblossom @pmunin and others