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

removeFile plugin API for allowing plugins to participate in removeCombined #344

Open
jrburke opened this issue Jan 12, 2013 · 3 comments
Open
Milestone

Comments

@jrburke
Copy link
Member

jrburke commented Jan 12, 2013

Suggested by @peol in requirejs/text#33: allow plugins, like the text plugin, to remove resources that are part of a build.

Maybe add a new build API a plugin can implement to do this. First thought:

removeFile(pluginId, resourceId, remove) where:

  • pluginId: String. Plugin ID
  • resourceId: The normalized resource ID
  • remove: Function. Pass it a string of a file name to remove.

Not sure if this is enough. For instance, a require.toUrl maybe useful?

It should be called when removeCombined is in effect, and it would need docs on the plugins.html page.

redoPop added a commit to redoPop/require-handlebars-plugin that referenced this issue Jun 2, 2013
The removeCombined hack breaks builds that reference the same
template multiple times in different modules, since the template
gets deleted on the first encounter.

Removing the hack in anticipation of r.js 2.2.0's removeFile API
per requirejs/r.js#344 which will provide a more robust standard
way of offering removeCombined support.
@spalger
Copy link

spalger commented Sep 20, 2013

I personally don't see why each plugin would need to implement the removal logic separately. I think it would be simpler if new property functions were included on write, maybe:

  • write.fileContents(filePath|filePath[], contents)
  • write.fileContentsAsModule(filePath|filePath[], moduleName, contents)

Then when r.js is wrapping up it's optimizations it can unlink all the filesPaths it received following the same logic it uses for removing combined scripts. From the plugin author's perspective it wouldn't require too many changes. In the context of the text plugin it could look something like this:

finishLoad: function (name, url, strip, content, onLoad) {
  ...
  buildMap[name] = {
    filename: url,
    content: content
  };
  ...
},
...
write: function (pluginName, moduleName, write, config) {
  ...
  write.fileContentsAsModule(
    buildMap[name].filename,
    pluginName + "!" + moduleName,
    "define(function () { return '" +
      content +
    "';});\n"
  );
  ...
}

It might even make sense to just add filePath as an optional argument to write and it's siblings...

Based on feedback I'd be happy to work this into a pull request. Disclaimer, I am not _at all_ familiar with the magic which occurs inside r.js, but I do ❤️ it.

@jrburke
Copy link
Member Author

jrburke commented Sep 23, 2013

The problem is that plugin dependency IDs are opaque to the loader -- it does not try to interpret what they mean. For a given dependency ID like 'pluginId!something-here', the 'something-here' part could be anything, a pointer to a more than one file, in the case of a 'has!some-test?ok.html:notok.html' case, or not even a file, like something that waits for an onload event from an img tag.

@spalger
Copy link

spalger commented Sep 25, 2013

This API would only be used for plugins that do resolve to file(s) and would just be a simple way for them to declare that "I included this file so it can be deleted if that's what the user wants".

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