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

added option to only write out files that have unused dependencies #5

Merged
merged 3 commits into from Apr 21, 2014

Conversation

Projects
None yet
2 participants
@tomwayson
Contributor

tomwayson commented Apr 17, 2014

In my workflow, when using the removeUnusedDependencies option, I only want to write out the files that have unused dependencies. This is because I use this plugin as more of a one-time or periodic check for unused dependencies, and when I find them, I want to remove them from the source files (instead of removing them each time I do a build). I do this by configuring this plugin to write the files to a temp directory and then copying the output files back over my source files. However, I don't want to overwrite any files that do not have unused dependencies, so I added an option to check for that before writing the files out.

By default, the option is set to false, so if you don't explicitly include it in your task configuration, the default behavior (write out all files) will be applied. To only write out files w/ unused dependencies, set onlyFilesWithUnusedDependencies: true as follows:

              options: {
                // logUnusedDependencyNames: true,
                removeUnusedDependencies: true,
                excepts: ['module'],
                exceptsPaths: ['require', /^jquery\./],
                onlyFilesWithUnusedDependencies: true
              },

NOTE: I did not yet document this in README.md. If you want to merge this, I can do that. Also, I did not update package.json version info.

@mehdishojaei

This comment has been minimized.

Show comment
Hide comment
@mehdishojaei

mehdishojaei Apr 18, 2014

Owner

I don't want to overwrite any files that do not have unused dependencies

Why?

Owner

mehdishojaei commented Apr 18, 2014

I don't want to overwrite any files that do not have unused dependencies

Why?

Show outdated Hide outdated tasks/amdcheck.js
@@ -105,7 +106,9 @@ module.exports = function(grunt) {
}
if (options.removeUnusedDependencies) {
grunt.file.write(dest, processResult.optimizedContent);
if (!options.onlyFilesWithUnusedDependencies || (results.length > 0 && fileHasUnusedDependencies)) {

This comment has been minimized.

@mehdishojaei

mehdishojaei Apr 18, 2014

Owner

I think there is no need to check for results.length > 0, right?

@mehdishojaei

mehdishojaei Apr 18, 2014

Owner

I think there is no need to check for results.length > 0, right?

@tomwayson

This comment has been minimized.

Show comment
Hide comment
@tomwayson

tomwayson Apr 18, 2014

Contributor

As for why I don't want to overwrite files w/o unused dependences, it's because amdextract modifies the contents of those files (related to the separator issue).

Even if that were fixed, and the contents of the output file were exactly the same as the input file, I'd still prefer not to overwrite source files that don't need it and would be more comfortable using this plugin if there were an option to prevent that. I would imagine others would feel the same way, no?

Contributor

tomwayson commented Apr 18, 2014

As for why I don't want to overwrite files w/o unused dependences, it's because amdextract modifies the contents of those files (related to the separator issue).

Even if that were fixed, and the contents of the output file were exactly the same as the input file, I'd still prefer not to overwrite source files that don't need it and would be more comfortable using this plugin if there were an option to prevent that. I would imagine others would feel the same way, no?

@mehdishojaei

This comment has been minimized.

Show comment
Hide comment
@mehdishojaei

mehdishojaei Apr 19, 2014

Owner

Even if that were fixed, and the contents of the output file were exactly the same as the input file, I'd still prefer not to overwrite source files that don't need it and would be more comfortable using this plugin if there were an option to prevent that. I would imagine others would feel the same way, no?

My be!

Before merging, i think the name of the option is a bit confusing: onlyFilesWithUnusedDependencies.
What about saveFilesWithUnusedDependenciesOnly?

About documentation you can go for that. I will update package.json after merging.

Owner

mehdishojaei commented Apr 19, 2014

Even if that were fixed, and the contents of the output file were exactly the same as the input file, I'd still prefer not to overwrite source files that don't need it and would be more comfortable using this plugin if there were an option to prevent that. I would imagine others would feel the same way, no?

My be!

Before merging, i think the name of the option is a bit confusing: onlyFilesWithUnusedDependencies.
What about saveFilesWithUnusedDependenciesOnly?

About documentation you can go for that. I will update package.json after merging.

changed option name to saveFilesWithUnusedDependenciesOnly and docume…
…nted in README

changed option name to saveFilesWithUnusedDependenciesOnly

added documentation for saveFilesWithUnusedDependenciesOnly
@tomwayson

This comment has been minimized.

Show comment
Hide comment
@tomwayson

tomwayson Apr 21, 2014

Contributor

Agreed, that name sounds better. I made the change and added documentation for it.

Thanks!

Contributor

tomwayson commented Apr 21, 2014

Agreed, that name sounds better. I made the change and added documentation for it.

Thanks!

mehdishojaei added a commit that referenced this pull request Apr 21, 2014

Merge pull request #5 from tomwayson/master
added option to only write out files that have unused dependencies

@mehdishojaei mehdishojaei merged commit c57e04c into mehdishojaei:master Apr 21, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment