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

Make public directory location configurable #56

Closed
rixo opened this issue Nov 24, 2019 · 16 comments
Closed

Make public directory location configurable #56

rixo opened this issue Nov 24, 2019 · 16 comments

Comments

@rixo
Copy link
Contributor

rixo commented Nov 24, 2019

I'm sorry to keep bugging you with all my issues, but I'd really like to bring Nollup to feature parity with Rollup, at least for the official Svelte template and simple cases, and make it the preferred solution for Svelte HMR. I just love Nollup's speed. Sometimes, I wonder if an update has applied because it happened in the time interval while my eyes were jumping from the left side of my screen to the right (editor -> browser)...

So, I've got a problem in my template because I haven't enough control (or maybe understanding) on the URL of the files generated by the bundling.

This problem is the reason why I have this special handling of output.file for Nollup. Note that this workaround doesn't work anymore with 0.9.0.

Say I have output.file: 'public/build/bundle.js'.

In versions before 0.9.0, Nollup will use the full path as URL, e.g. http://localhost:8080/public/build/bundle.js.

In 0.9.0, it only uses the basename, e.g. http://localhost:8080/bundle.js.

What I really want is http://localhost:8080/build/bundle.js. That's what I get if I build this with Rollup and launch a http server from the public directory. The file gets written in public/build/bundle.js, and the server sees build/bundle.js as the URL.

So I would expect Nollup with contentBase: 'public' to produce the same URL.

For illustration, I've hacked something that works for me.

I think reusing contentBase is the correct behaviour because, like I said, that's what you get if you use Rollup + a web server. So Nollup, to align with Rollup, would write the "virtual" (in-memory) file to public/build/bundle.js; and, if it serves (contentBase) from dir public, then it should see the URL as build/bundle.js. Is this a good idea to change the meaning of an existing variable though?

Another question is: what to do with files that are written outside of the public directory? For example, if I have output.file: 'dist/bundle.js', but contentBase: 'public'. Hmm. In this case, my implementation just use the full path of the file as URL, because I don't see any other solution.

With output.dir, I think Nollup's behaviour is aligned with Rollup, as a bundler. However, as server, it makes the assumption that the output dir is also the public dir, which might not hold in every project. I guess most cases can be handled by tweaking entryFileNames, but I think that these settings should live in Nollup's config, not Rollup's. They are related to serving, not bundling.

I'm not too sure what the solution should look like. Should it be based on contentBase or some new specific options?

@PepsRyuu
Copy link
Owner

PepsRyuu commented Nov 26, 2019

Haha, thanks! Pretty sure I can still increase the performance!

From reversing Rollup, the behaviour that 0.9.0 matches Rollup in terms of the Rollup API. When you run a build using Rollup using rollup.rollup(), and then run bundle.generate(), it will return an array of objects with the fileName property. That only provides the basename, not a full directory, when using the file option.

However! If you use entryFileNames, it will preserve the entire path as specified and reflect it in fileName. Nollup's behaviour also matches Rollup here.

If using the dir option, neither Nollup or Rollup append that to fileName.

As far as I can tell, Rollup and Nollup are aligned here. The core difference is that when you're using Rollup in that scenario, it's using bundle.write() which has a different behaviour, and isn't implemented by Nollup (as it never writes to disk). Is it possible that the solution here is something like Webpack's publicPath option which allows you to remap where content is served from? https://webpack.js.org/configuration/dev-server/#devserverpublicpath-

To clarify:

  • contentBase - Directory which contains static content to serve.
  • publicPath - The base URL to serve content.

@rixo
Copy link
Contributor Author

rixo commented Nov 26, 2019

it's using bundle.write() which has a different behaviour

That must be it then. I can't really tell about the programmatic API, never really used it yet. So no problem for me on this side!

To clarify:

contentBase - Directory which contains static content to serve.
publicPath - The base URL to serve content.

publicPath, as per Webpack, is prepended to the baseName to form the URL, if I understand correctly? I would have called that baseUrl. It would solve my own use case in Svelte template.

But what about multiple output files?

  output: [
    { file: 'public/app/index.js', format: 'es' },
    { file: 'public/vendor/index.js', format: 'es' },
  ],

How do I set publicPath so that both assets are accessible from respectively /app/index.js and /vendor/index.js in the browser?

Also, if Nollup completely drops the parent directories, one of them will not be accessible because they will have the same index.js URL. As you said, this is the part that is not aligned with Rollup, because Rollup would write the directories to disk, and their path would be part of the URL when serving.

However if publicPath is taken as an actual FS path and URLs are computed as relative to it, then it works if I set it to public. The problematic case here is if you get an asset that falls outside of the designated publicPath... I'd say "not accessible by the web server" in this case.

What approach were you thinking about?

@PepsRyuu
Copy link
Owner

If publicPath was implemented, it wouldn't allow to serve from different directories.

Is there any particular reason entryFileNames can't work here?

@rixo
Copy link
Contributor Author

rixo commented Nov 27, 2019

If publicPath was implemented, it wouldn't allow to serve from different directories.

Maybe contentPath for "fs path to the public directory", to mirror contentBase, and to avoid having an option with the same name in Webpack but with a different behaviour? But it might be confusing... Maybe publicDir or public?

Is there any particular reason entryFileNames can't work here?

I hadn't really dug into it, to be honest. But now that I try, I get mixed results.

I manage to get what I want, that is to have my bundle served at /build/bundle.js (in both Rollup & Nollup), with this config:

export default {
  input: './src/main.js',
  output: {
    dir: 'public/build',
    entryFileNames: 'build/bundle.js',
  },
  plugins,
}

I fail however to get it working with output.file. Whatever I tried, I can only get the bundle file served from root /bundle.js.

I've pushed a new branch to my test repo (will need to rename that playground) that illustrates what I'm doing with Rollup and failing to achieve with Nollup.

Do you have any suggestion on how to make it work?

As a side note, it seems that Nollup has no support for multiple output targets (see example on another branch). Still possible that I'm missing something obvious, though... For a real world example, this is used in Svelte's component-template.

Anyway... Support for output.file is important to me because I want to remain as close to the official Svelte template as possible, and it uses output.file. Personally, I can also see the value in it. output.file is obvious to understand, just by looking at it. On the other hand something like entryFileNames is not so obvious, and probably even a little bit scary for some people. In any events, it probably warrants a read in the docs, if you want to understand what your rollup.config.js does, let alone tweak it. But what docs? Where docs? With output.files, these difficulties for newcomers can be entirely deferred until it's a better time for them to learn more about Rollup. (This is far less of an issue with the Webpack config, where the whole file already scares most people away, and nobody will really try to read & understand the details... Hu hu, just trolling.)

That being said, since a workaround is possible with dir + entryFileNames, this is not a deal breaker for me. I can live with a little bit of Nollup specific config, if you're not interested in solving this for now.

@PepsRyuu
Copy link
Owner

Hmmm, I was thinking about this throughout the week, and here's some thoughts I had:

  • For Nollup CLI/Middleware, file will behave as if it was using bundle.write(), which means that it will output to the full path. bundle.generate() will behave as it currently does with just the filename. This can be done by the middleware very easily without having to modify the core.

  • Multiple output formats can be supported to output to multiple module formats. Not sure about output specific plugins, but can definitely support multi-format output.

  • It seems one of the goals here is supporting bundling a vendor bundle with third party dependencies. One feature that I haven't implemented yet is manualChunks: https://rollupjs.org/guide/en/#manualchunks along with emitChunk. It's a bit more complex in Nollup's context than it is for Rollup (what exactly the bundle structure should look like), but it is something I would like to get done.

@rixo
Copy link
Contributor Author

rixo commented Dec 1, 2019

I agree with your first point. Since that's mainly a serving issue, it seems logical that it should be addressed at the middleware level.

The question remains though: what will be the URL for a file that is written under the public directory (the one setup as contentBase)? And subsidiary: what would be the effect for files written outside of the contentBase dir?

It seems one of the goals here is supporting bundling a vendor bundle with third party dependencies.

Actually, I think it was more a bad example on my part. It wouldn't be possible to do that with Rollup this way either, would it? Anyway, that's not really a need I have, I just noted the misalignment with Rollup while exploring for the public dir issue. I don't have a solid opinion about what should be done in this area.

@PepsRyuu
Copy link
Owner

PepsRyuu commented Dec 1, 2019

That's a good question, and it was something I was wondering.

A typically project would have something like the following folder structure:

dist/
    index.html
    bundle.js
public/
    index.html
src/
    main.js
rollup.config.js

With the contentBase option, it will serve everything in the specified directory as if it was coming from /. This will allow you to have /index.html working. For the bundle, with entryFileNames there's no problem with this structure, because Nollup ignores the dir option which is typically set to dist. That means you get your bundle at /bundle.js

The problem then comes when you use file. You might specify it as dist/bundle.js. With the above proposal, it would be served at /dist/bundle.js. This can work fine, but it means that you have to change how your directory structure works. public in most projects is usually treated as a source directory that should be free of build artifacts, so file really should not be pointing to it. Not sure how I feel about auto-detecting contentBase in file and stripping it out.

I've been looking into Webpack as to how they've addressed this, but this seems like this is an exclusive problem for Rollup configs. Webpack has a separation of output directory and bundle filename options, rather than a single option. So there's no precedent to follow as far as I can tell.

There is this: https://github.com/rollup/rollup-starter-app, I don't think it's a great pattern because it's mixing build artifacts with source code (they have added a gitignore rule for this though). So maybe it has to be one of those things where contentBase has to be auto-detected in the file output path.

@rixo
Copy link
Contributor Author

rixo commented Dec 2, 2019

public in most projects is usually treated as a source directory that should be free of build artifacts, so file really should not be pointing to it.

I would have agreed with you wholeheartedly... But, look who did it, and again, and when pushed about it... He doubles down. Clearly, there's something going on here.

I'm not in his head, but I think he does all this in the sake of reducing complexity for newcomers. Like with file vs dir (+ entryChunkNames or whatever), the concept of a content root directory is easy to grasp for anyone with experience with a web server. What you see in public is what you get in the browser, irrelevantly of the file's origin (whether it just sits there like public/index.html, or has been written by the bundler). Stupid simple. Whereas, with a dist folder, you've got to understand that you have build steps meant to copy assets, etc.

Rich seems all bent on the easiest possible first contact for beginners, followed by an optional and postponed increase in complexity, as per the needs & desire of the user. Each of these are little things when taken individually. But, as a whole, they end up making the difference between Webpack's API and Rollup's.

Since this pattern has been consistently endorsed and popularized by Rollup's author for years, I feel compelled to take my own opinion with a pinch of salt here. At the very least, I think it means the pattern should be supported.

Not sure how I feel about auto-detecting contentBase in file and stripping it out.

I think the higher level final goal is pretty well defined: since Nollup acts as Rollup + a web server, it should behave like Rollup + a web server would do.

A web server would serve all files in its "web root" directory, with URLs being the path of each file relative to the public directory.

I can't think of a straightforward way of implementing this, except by doing exactly the same thing. That is (1) to make Nollup generate the same file paths in its head (memory) as Rollup would have written to disk, and (2) to compute the URL as this path relative to the public dir.

The benefit of this approach is that it deconflates the responsibilities of generating output filenames vs generating URLs. This way, you don't have to think & test URLs as a function of output options. Rather, you can implement & test them independently of each other. On one hand you've got fs paths as a function of output options: they're good when they're the same as Rollup with the same options. On the other hand, you've got URLs as a function of fs paths. And when the specs of one or the other will change, for example because someone ask for support of multiple public directories, only the relevant part will be affected.

@rixo
Copy link
Contributor Author

rixo commented Dec 10, 2019

Hey!

Unfortunately, I'm not sure I'll have time to write a proper issue for the buildStart thing this week or early this week (don't have time to do the research to be sure what's really needed...).

But I just wanted to let you know that this issue is currently more important for me. I am reluctant of introducing entryChunkNames in Svelte's hot template, but without doing so I can't currently upgrade Nollup past version 8.x... That puts me in a bit of a dilemma.

Like I said previously, I've got ways to live with it, so I don't mean to press you by any means. And I still have to fix support for last Svelte version on my side... On the contrary, I want to thank you for all the help and the work you've done so far! Just letting you know about my own current priorities, in case that's useful for your decision making ;)

@PepsRyuu
Copy link
Owner

Oh don't worry about it at all. I love hearing about all of the issues as it helps to make the project work better for people. 🙂 This issue is the first on my agenda that should hopefully be addressed this week! 😁

@PepsRyuu
Copy link
Owner

https://github.com/PepsRyuu/nollup/tree/FileOptionServe

Dev middleware takes care of the following now:

  • Array support for output. This is aligned with Rollup CLI.
  • Output file to full path, and remove contentBase from the file.

Going to be writing test cases for dev middleware. Let me know what you think!

@rixo
Copy link
Contributor Author

rixo commented Dec 28, 2019

Sorry for taking so long to come back at this... Totally got sucked into "must deliver before Christmas" and stuff!

So, reusing contentBase? I like that, it seems like the more straightforward way to mimic a real web server. I guess you'll probably need some kind of support for multiple public directories for completeness, in the future. But I personally haven't encountered a real use case yet, so it might be worth waiting for one.

It feels a bit like your surfing the tip of the happy path with this but I can't think of a case where it wouldn't work, so heh...

I tried the branch in the Svelte template. Works great for my current needs, that is output.file. With this change and the previous one of setting NOLLUP env variable automatically, the same config can essentially be used transparently with Nollup & Rollup \o/

For more advanced usages though (dynamic imports / code splitting next in line), it seems that Rollup and Nollup are not aligned on URLs with output.dir. For example, with this config:

input: 'src/main.js',
output: {
  format: 'esm',
  dir: 'public/build',
}

Rollup will write everything under public/build/, for example public/build/main.js; and so the file would be accessible at URL /build/main.js.

However, with Nollup (and --content-base ./public), the file will be accessible at URL /main.js.

Adding entryFileNames: 'build/[name].js' to the above config makes the file accessible at /build/main.js with Nollup. But, this time, Rollup would write entry points under public/build/build/, hence serving it at URL /build/build/main.js. Same with chunkFileNames, as far as I can tell.

So it appears that URL-wise, we cannot currently get same result with same config between Rollup + "serve public" and Nollup for output.dir? My opinion is this should be aligned on Nollup side, mirroring output.file behaviour.

@PepsRyuu
Copy link
Owner

It's an interesting point, but I don't think doing the same thing with dir would be correct course of action.

The way I see dir is that it intended to be used for directories such as dist, which would also contain content from public. In this situation, the URL being /dist/main.js would be incorrect. Nollup mimics the output.path option in Webpack for this, where webpack-dev-server completely ignores it and it's only considered when building for production.

As far as I can tell, prefixing URLs with the dist value would break apps that put there bundles inside dist instead of public. Even if the developer using Rollup/Nollup believes that artifacts should be generated into public, it's still possible to get them both working cleanly:

output: {
    dir: 'public',
    entryFileNames: 'build/[main].js'
}

@rixo
Copy link
Contributor Author

rixo commented Dec 29, 2019

Even if the developer using Rollup/Nollup believes that artifacts should be generated into public

Hu hu. Nice wording.

It might actually be a use case for multiple public directories. Or dist would be the (single) public directory. In any case, as the content base, it would be dropped from the URL, and would resolve correctly to URL /main.js and not /dist/main.js.

My gut feeling, even knee jerk reaction, is that Webpack is uselessly complicated on this point. I remember having a hard time wrestling with this precise set of options. My intuition is that a simple no-exception rule like "everything relative to the public dir(s)" would be easier to comprehend.

That being said, truly I don't know. I really don't have a strong opinion on this... If there is a way to have both Nollup and Rollup behave identically with the same non-franken-hackish config, I'm happy with that. I can take it as the "proper" way.

I had overlooked the config you're suggesting, but for me it fits the bill. So in regard to this issue, that is generated URLs, I think I'm all good with the proposed PR, both with output.file and output.dir.

I'll need to dig more in the question of dynamic imports, but so far so good. I'll open another issue, if needed.

Thanks again for all your help :)

@PepsRyuu
Copy link
Owner

PepsRyuu commented Jan 6, 2020

Released in 0.10.0 :)

@PepsRyuu PepsRyuu closed this as completed Jan 6, 2020
@rixo
Copy link
Contributor Author

rixo commented Jan 6, 2020

Works like a charm, thanks! (rixo/svelte-template-hot@442e638)

Now, in the config, I only need the NOLLUP env variable to prevent firing up the web server. I'm not a big fan that they put the web server in the Rollup config... The config doesn't need the variable anymore because it is now used internally by the plugins to handle the differences, which is made safe by the variable being official :)

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