neutrinojs / neutrino

Create and build modern JavaScript projects with zero initial configuration.
https://neutrinojs.org
Mozilla Public License 2.0
3.94k stars 213 forks source link

Copying static assets to places other than neutrino.options.output/static #290

Closed timkelty closed 7 years ago

timkelty commented 7 years ago

Currently when copying static assets, we copy like so: to: basename(neutrino.options.static)

This means if we have [source]/static/foo.bar it copied to [output]/static/foo.bar: https://github.com/mozilla-neutrino/neutrino-dev/blob/master/packages/neutrino-preset-web/index.js#L160

This makes it impossible to copy anywhere but [output]/static, which may not be ideal.

For example, you may wish to copy the contents of [source]/static directly to neutrino.options.output for files you might want at the root of your output (e.g. robots.txt, manifests).

Changing the to to neutrino.options.output would give users more flexibility.

Anything in source/static would get copied, so if you wanted it in output/static/foo.bar you would put it in source/static/static/foo.bar. source/static/robots.txt would get copied to output/robots.txt.

Configurable option

And/or it would be nice if something like staticOutput were a configurable destination. Consider this config:

{
  options: {
    source: './app',
    output: './public/assets',
    entry: './js/index.js',
  }
}

Now I want a static file (e.g. robots.txt) copied to ./public (not ./public/assets). If we were able to configure staticOutput or something similar, we could set that to ./public and all would be well.

I can work on a PR but I just wanted to get this out there for input.

constgen commented 7 years ago

Static functionality shouldn't be touched. This is a completely another middleware that does copy.

timkelty commented 7 years ago

This is a completely another middleware that does copy.

I realize that, but I'm specifically talking about the options passed to it, which is done in this preset.

Most of all I think the basename/to bit is awkward and ultimately ends up being more restrictive.

timkelty commented 7 years ago

A non-breaking way of doing this could be to have option.static be able to remain as a string, or optionally you could pass an object:

options.static = {
  source: './static',
  output: path.join(neutrino.root, 'public'),
}

If you passed a string, the behavior would remain as is.

eliperelman commented 7 years ago

You bring up a good point: every source has a destination. For neutrino.options.source, this is neutrino.options.output. For neutrino.options.static, the destination isn't overridable, so maybe we should also have neutrino.options.staticOutput.

Or maybe we completely remove the static options altogether since it causes a lot of problems, and we let users configure this as needed.

timkelty commented 7 years ago

Or maybe we completely remove the static options altogether since it causes a lot of problems, and we let users configure this as needed.

That's kind of what I'm thinking. Seems like unnecessary overhead. Then we wouldn't even need options.static as an API level option, which simplifies things.

Maybe removing static can be a proposal for v7, and as a non-breaking stop-gap for v6 we can add neutrino.options.staticOutput as an option?

barraponto commented 7 years ago

I like static copying -- a lot better than what we had before, and somewhat better than setting up the copy myself...

timkelty commented 7 years ago

@barraponto fair enough. Perhaps we should just try to make it a little more flexible before deciding to axe it.

To recap, here are some options:

Change the to passed to neutrino-middleware-static to just neutrino.options.output

This seems simplest and most logical to me, but would be a breaking change.

This would give users full control of where things go within neutrino.options.output. This way we wouldn't need a separate staticOutput option.

Result: anything in neutrino.options.static gets copied to neutrino.options.output.

I.e. if you wanted things in ${neutrino.options.output}/static like current implementation, simply put files in ${neutrino.options.static}/static

Add new api option for neutrino.options.staticOutput

Would be non-breaking, probably the easiest implement.

Allow neutrino.options.static to be an object

    static: {
      source: './static',
      output: path.join(neutrino.root, 'public'),
    }

Remove static from presets, deprecate neutrino.options.static.

Tradeoff is we're simplifying core api/middlewares, but complexifying users config that need static copying.

To replicate the exiting functionality, one would have to do something like:


  return {
    use: [
      'neutrino-preset-web',
      ['neutrino-middleware-static', {
        patterns: [
          {
            context: neutrino.options.static,
            from: '**/*',
            to: basename(neutrino.options.static),
          },
        ]
      }],
    ]
  }
}```
eliperelman commented 7 years ago

Technically the last option is already possible with Neutrino, although it is just the copy middleware:

module.exports = {
  use: [
    ['neutrino-middleware-copy', {
        patterns: [{
          context: neutrino.options.static,
          from: '**/*',
          to: basename(neutrino.options.static)
        }],
        options: {}
    }]
  ]
};
eliperelman commented 7 years ago

The problem with making static map to output is relative paths during development. The URLs wouldn't map the same from dev to production.

By that, I mean src/static -> output would cause URLs to change.

constgen commented 7 years ago

As for me the problem is overcomplicated. It should be stupid simple. Leave static middleware is it is. Create a new neutrino-middlware-favicon or what ever it is called and use it on top of existing preset. The issue will be resoved. I don't like the idea to remove static resources copying just because we didn't think about a favicon. Also I would suggest to think not only about a favicon but also all variety of icons for deifferent platforms. This requires also an integration with html-webpack-plugin

timkelty commented 7 years ago

I don't like the idea to remove static resources copying just because we didn't think about a favicon.

FWIW the first use case I had for this was actually for copying some files that needed to be in my output root for authoring a Chrome extension.

constgen commented 7 years ago

Above we discussed how to exclude it from Neutrino. I can't agree with this.

Remove static from presets, deprecate neutrino.options.static.

eliperelman commented 7 years ago

Right now I think my previous comment is the most reasonable solution. We can still include it in the web and node presets, and default its output to src/static, with any overrides obviously changing that.

barraponto commented 7 years ago

@timkelty we definitely need a web extension preset.

eliperelman commented 7 years ago

@barraponto #306 :)

eliperelman commented 7 years ago

This is now resolved in Neutrino v7 which removed the static option, but makes copying more of a manual process. Still will copy from src/static, but overridable with using the copy middleware.

bradsimantel commented 6 years ago

Is this issue's resolution documented somewhere? I stumbled across this issue while trying to figure out where to put favicon.ico and robots.txt and, looking at the copy middleware, I can't figure it out.

eliperelman commented 6 years ago

@bradsimantel this is documented in the web/react/etc. presets:

Static assets

If you wish to copy files to the build directory that are not imported from application code, you can place them in a directory within src called static. All files in this directory will be copied from src/static to build/static. To change this behavior, specify your own patterns with @neutrinojs/copy.

In a nutshell, it says that anything placed in src/static gets copied to build/static, and you need to use the copy middleware to override. In your use case, you probably want those copied to the build root, so something like this in your .neutrinorc.js (if you're using the react preset for example):

const { basename, join } = require('path');

module.exports = {
  use: [
    '@neutrinojs/react',
    ['@neutrinojs/copy', {
      patterns: [{
        context: join(neutrino.options.source, 'static'),
        from: '**/*',
        to: basename(neutrino.options.output)
      }]
    }]
  ]
};
bradsimantel commented 6 years ago

@eliperelman Got it, thanks!