Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

Write app to src/node_modules/app? #551

Closed
Rich-Harris opened this issue Jan 24, 2019 · 57 comments
Closed

Write app to src/node_modules/app? #551

Rich-Harris opened this issue Jan 24, 2019 · 57 comments

Comments

@Rich-Harris
Copy link
Member

Summarising an earlier conversation from Discord for posterity, for when Svelte 3 is done and the focus can return to Sapper.

A lot of people find it a bit weird (where 'weird' means 'ugly', 'hacky-seeming', 'awkward' or whatever) that you need to import the generated Sapper app from ../__sapper__/client.js and so on. It's especially bad if you need to import (say) goto from a deeply nested file — ../../../../../.

One solution would be to use aliases in the webpack and Rollup configs. This has a couple of drawbacks:

  • it hides important implementation details, making it harder for someone coming to the project to understand where the app is 'coming from'
  • it adds further complexity to the build configs, increasing maintenance burden (e.g. if we were to add support for Parcel etc)

An alternative idea would be to write the app to node_modules inside the app's src directory — i.e. src/node_modules/app/client.js and so on:

// src/client.js
-import * as sapper from '../__sapper__/client.js';
+import * as app from 'app';

-sapper.start({
+app.start({
  target: document.querySelector('#sapper')
});
// src/server.js
import sirv from 'sirv';
import polka from 'polka';
import compression from 'compression';
-import * as sapper from '../__sapper__/server.js';
+import * as app from 'app';

const { PORT, NODE_ENV } = process.env;
const dev = NODE_ENV === 'development';

polka() // You can also use Express
  .use(
    compression({ threshold: 0 }),
    sirv('static', { dev }),
-    sapper.middleware()
+    app.middleware()
  )
  .listen(PORT, err => {
    if (err) console.log('error', err);
  });

(You'll note I've imported app instead of app/client.js and app/server.js — I'm wondering if it's possible to have a single module and rely on tree-shaking, or if that would slow down the build too much.)

To me this feels like all upside:

  • it's very easy to type, and reinforces the idea that 'app' is a singleton that is bespokely generated for you
  • everybody pretty much intuitively understands the node_modules resolution algorithm, so it wouldn't be surprising behaviour
  • it's visible — you can see where your app is coming from, and can read the generated source code in the app folder to understand how everything works (e.g. what the route manifest looks like)
  • node_modules is already gitignored, so it's unobtrusive

A few people are a bit sceptical of the idea, and have raised points such as:

  • maybe ../../../../__sapper__/client.js isn't so bad
  • it's weird to put generated code in a node_modules directory
  • it's weird to put a node_modules directory in src
  • maybe app is the wrong name (what if I wanted to use https://www.npmjs.com/package/app in my Sapper app?!)
  • dammit Rich, we only just changed it from sapper/runtime.js and ./manifest/client.js. stop breaking our apps you feckless idiot

It has to be src/node_modules and not node_modules, since npm and yarn are likely to periodically nuke generated code in node_modules. I'd argue that's preferable anyway, for the visibility reason stated above.

What does everyone think?

@ghost
Copy link

ghost commented Jan 24, 2019

What about just fiddling with the NODE_PATH so that it can resolve modules from __sapper__?

@Rich-Harris
Copy link
Member Author

I don't think that'll work with Rollup, not sure about webpack. Either way though it's basically another form of aliasing which means it has the same drawback of hiding implementation details, which runs counter to Sapper's 'show your work' philosophy. I think there's a lot of value in people being able to see how their application is constructed, rather than keeping it in a black box

@timhall
Copy link

timhall commented Jan 24, 2019

2 things:

  1. It might be nice to take the svelte v3 approach with lifecycle hooks and have goto loaded directly from sapper import { goto } from 'sapper'
  2. To avoid conflicts, you could create/camp on a sapper organization and import * as app from '@sapper/app'

Using a nested node_modules is what PouchDB used when it switched to a monorepo / dropped lerna, so it's not an unheard of approach. I like it!

@Rich-Harris
Copy link
Member Author

re import { goto } from 'sapper', it's not exactly like Svelte 3 lifecycle hooks, in that the implementation of goto depends on your app's manifest. There's two ways to achieve that — one is to require the app author to register the manifest before using any Sapper functions (which is what used to happen, clunkily), the other is to make goto part of your app, which is what currently happens (and would continue to happen under this proposal).

So it needs to be import { goto } from 'app' or import { goto } from '@sapper/app' or whatever.

Interesting to read that PouchDB thing

@arxpoetica
Copy link
Member

I like this idea, even if it's weird.

I made a little bit of a rabble about cache busting in the Discord channel. I would say as long as sapper build etc. do a good job cleaning things up, this should be all good.

@tbillington
Copy link

Bit of a 'potential' issue that is unverified, some build systems/plugins/tools may look check for something being in/out of node_modules through a regex on the path or similar check, and may incorrectly flag the existence of node_modules in the path at all as being the actual 'node_modules'. Probably not a 'hard' problem but it could cause weird tooling/ecosystem issues intermittently ?

@tomcon
Copy link

tomcon commented Jan 25, 2019

Seems to me like a fine idea.
Only change I'd make is to name(space) the app somewhere, somehow. Package.json??
That way, anyone else thinking this is a cracking idea (to use app subdir) won't be trodden on/cause complications

@ansarizafar
Copy link

ansarizafar commented Jan 25, 2019

An SSR capable Code base router like vue-router provide more freedom and options to define routes than file-based routing. We should consider switching to code based router. We can use router api (defineRoutes, goto) to define routes in code and then pass the router to sapper.middleware on the server and sapper.start on the client.

@Rich-Harris
Copy link
Member Author

@tbillington makes a good point — definitely something to watch out for. I think most tooling probably checks if a path is in [cwd]/node_modules but can't rule out some tools not working that way.

@tomcon The easiest way to control it is probably to expose a CLI flag. We already have --output which defaults to __sapper__; if it defaulted to src/node_modules/@sapper/app instead we'd get the desired behaviour essentially for free, and people could easily a) choose a different name or b) opt out of the mechanism altogether.

@ansarizafar

We should consider switching to code based router

You should consider switching to a different framework 😉 The whole point of Sapper is that it provides the ergonomic affordances of fs-based routes. Code-based makes SSR a lot harder, and component-based makes code-splitting a lot harder.

@ansarizafar
Copy link

It's difficult to switch to a different framework If you have used amazing Svelte/Sapper but no framework is perfect. every framework has its own merits and demerits. SSR capable code based router is on my wishlist for Svelte.

@Rich-Harris
Copy link
Member Author

This is working in the Svelte 3 PR, #538. @sapper/client doesn't really make sense since some of its exports are used during SSR, so for now I've gone with

  • @sapper/app — exports start, goto, prefetch and prefetchRoutes (as before), getSession (the topic of this issue), preloading and page (readonly stores — see Turn params, query, preloading and page into stores #546)
  • @sapper/server — as before, exports middleware (in time, should export a render function as well)
  • @sapper/service-worker — also unchanged

It'd be great if they could all be a single package but server isn't tree-shakeable, and service-worker includes hashed filenames of the chunks that result from app, so it's not possible

Rich-Harris added a commit that referenced this issue Feb 3, 2019
@jakobrosenberg
Copy link

Newcomer here.

I know I'm late to the party but here's my experience.

I only found out about Svelte and Sapper a few days ago and was immediately drawn to the cleanliness. However... having just discovered a node_modules folder within my src folder, I spent the last hour reviewing my code trying to troubleshoot how it ended up there.

Aliasing the Sapper folder might add a bit of complexity to the build config, but it's much better than adding complexity to the main workspace by poluting src with generated code. At least with a build config, the complexity stays where it belongs.

Convention over configuration was what sold me on Sapper and I can't believe intuitiveness and convention got sacrificed for the purpose of a slimmer build config file. I'm baffled.

@pngwn
Copy link
Member

pngwn commented Aug 11, 2019

Having additional node_modules folders and referencing the files within via import x from 'file' is a lot less complex than any other solution as it makes use of a convention that is present in every single node application. It simply uses the existing node_modules resolution rules.

@antony
Copy link
Member

antony commented Aug 11, 2019

@jakobrosenberg
Copy link

I'm talking about the elephant in the room. Generated readonly code in a src folder.

src is where I keep my own code; my workspace. The code I can change, be it scaffolded boilerplates or hand created files. If I want to change the output, I change the source. Instead, now I have source files I can't change, cause the source folder is also used as a build destination.

It's not just a convention, it's even in the name src = source. Src contains the source files for what you're generating. Not the other way around. I want to call it a structural antipattern, but it goes beyond that.

I have a great deal of respect for Svelte and Sapper and would love to use it, cause there's so much to love. For now I cross my fingers and hope that some day the source folder will become the place for source files.

@pngwn
Copy link
Member

pngwn commented Aug 11, 2019

They are source files; they are the files that are used to generate the built files. They are specific to your application, just like the source code you have written yourself. The only difference is Sapper generates them for you.

@jakobrosenberg
Copy link

The only difference is Sapper generates them for you.

The difference isn't that Sapper generated them. Generated files are common in src folders. Boilerplates created by CLIs etc.

The real difference is that they're internal readonly files that I can't work with. As such, all they do is clutter up the one space I'd like clutter to be abstracted away from.

@pngwn
Copy link
Member

pngwn commented Aug 11, 2019

Put a little piece of tape on your screen where the folder name is and it won't feel as cluttered.

@Rich-Harris
Copy link
Member Author

You can generally configure editors to hide gitignored folders

@jimjones26
Copy link

I have told vscode to ignore all folders I don't want to see. It's wonderful really, then I am not worried about having to look at them or why they are there.

@swyxio
Copy link

swyxio commented Sep 13, 2019

i have also suggested adding a README to the generated folder to ease the surprise i and @jakobrosenberg had.

@tbillington
Copy link

tbillington commented Sep 14, 2019

Seeing this thread reminded me I've seen a few tools around that go through your computer to delete all the node_modules folders to free up space. I've used one in the past and recovered +10gb.. so I see their value. I don't know if any of them would safely handle (leave alone) node_modules inside src though..!! Just putting this here to be documented in some form.

@jakobrosenberg
Copy link

For anyone who'd prefer to keep modules within./node_modules and not have ./src/node_modules, this did the trick for me.

Edit package.json and change
"dev": "sapper dev to
"dev": "sapper dev --output ./node_modules/@sapper"

Note: I still haven't figured why ./src/ is used by default, so I don't know if there could be unforeseen consequences. It does work with the default Sapper template.

@pngwn
Copy link
Member

pngwn commented Sep 25, 2019

Because it is app-specific source code, not an unchanging, reusable module. Putting the sapper code into the root node_modules will mean it get wiped out very regularly.

This may or may not break things.

@swyxio
Copy link

swyxio commented Sep 25, 2019

seems fine since @sapper is generated if it doesnt exist?

@moon-bits
Copy link

Indeed @necauqua, this was just an awful design-decision to put auto-generated code into the src/ folder.

I can't believe the argument of @Rich-Harris:

it adds further complexity to the build configs, increasing maintenance burden

Is the build-config going to change so often that the effort for maintaining it is going to be a burden? Heck, I fell immediately in love with Svelte and Sapper...and then this...

...Grandma was right, every time you think you are in heaven, reality pulls you back again.

@Rich-Harris
Copy link
Member Author

People, if you think it's preferable to do

import { app } from '../../../../../../__sapper__/client.js';

than to realign your perspective a bit on what constitutes source code, then... you're just wrong. I'm not sure there's a kinder way to put it. Sorry!

I'll often have scripts in all kinds of projects that fetch some data or create some module then write it into my src folder so that it gets the same treatment as the rest of my stuff. The interesting distinction isn't between stuff-i-wrote and stuff-i-didn't-write, it's between stuff-that's-specific-to-this-app-or-library and stuff-that-comes-from-elsewhere.

Cast off your preconceptions. Try it for a bit. Start putting common components in src/node_modules/@components and utils in src/node_modules/@utils. You will feel liberated.

@moon-bits
Copy link

moon-bits commented Jun 18, 2020

People, if you think it's preferable to do

import { app } from '../../../../../../__sapper__/client.js';

True, this is not a solution either.

The solution would be to include it within the build-config where it belongs. For now I just use the fix that @jakobrosenberg provided. Yet, I'm asking myself what the side-effects are.

Everything seems to be working...till now. So @Rich-Harris, what is the drawback of having this as default:

"dev": "sapper dev --output ./node_modules/@sapper"

@pngwn
Copy link
Member

pngwn commented Jun 18, 2020

In the build config where it belongs. As opposed to leveraging the default, built-in, thats-how-it-is-supposed-to-work, node_modules resolution behaviour?

@Rich-Harris
Copy link
Member Author

what is the drawback

The drawback is that the next time you do npm install something, any files that don't correspond to something in your package-lock.json — in other words the generated Sapper files — will be nuked. In general, a node_modules adjacent to a package.json is no-go territory.

Trust me, we don't make these decisions quickly or arbitrarily. We do know what we're doing! In fact I wrote this at the very top of this thread:

It has to be src/node_modules and not node_modules, since npm and yarn are likely to periodically nuke generated code in node_modules.

@grundmanise
Copy link

grundmanise commented Jun 18, 2020

@Rich-Harris I don't think it's appropriate to call people who have different perspectives "wrong". The issue here is that it's uncommon, unexpected, and undocumented behavior. node_modules was always (and still is) the place for external packages; The idea that a build tool outputs auto-generated app-specific-code inside of it is at least misleading IMO. That could be solved in a more common way by utilising aliases in the bundler config. You of course can use node_modules in whatever way suits you best, but if it's a framework aimed at mass adoption, and that approach diverges with a mental model of its consumers' then I'd reconsider that approach.

@pngwn
Copy link
Member

pngwn commented Jun 18, 2020

The issue here is that it's uncommon, unexpected, and undocumented behavior. node_modules was always (and still is) the place for external packages; The idea that a build tool outputs auto-generated app-specific-code inside of it is at least misleading IMO.

I'm afraid this is not true. This is precisely why the node_modules resolution algorithm works the way it does, to give you flexibility to leverage it in your source code. This is default behaviour and, while it might not be widely known, it is definitely expected.

@grundmanise
Copy link

@pngwn it's expected with external packages, but not with app specific code.

@Rich-Harris
Copy link
Member Author

node_modules was always (and still is) the place for external packages

Unfortunately this is ahistorical. The node resolution algorithm predates npm; early Node tutorials encouraged you to put your own modules in node_modules. npm simply piggybacked on that behaviour. Once the 'small modules' doctrine took hold, node_modules became an unruly place that was unsuitable for checking into version control, and we began the journey to where we are now — a place with lockfiles, and dedicated CI versions of npm install, and so on. But the node resolution algorithm is unchanged.

but if it's a framework aimed at mass adoption, and that approach diverges with a mental model of its consumers' then I'd reconsider that approach.

I don't like to simply work on projects to satisfy people's existing mental models, I like to use these projects to expand people's models of how code can and should work. The reliance on ugly hacks like adding aliases to our build configs (which then have to be duplicated for the benefit of typecheckers, linters, editors, etc) are an artifact of people not having general awareness of how things like node_modules are supposed to work. Changing that might be harder than simply adopting those brittle hacks, but the good fight is always harder.

@pngwn
Copy link
Member

pngwn commented Jun 18, 2020

@pngwn it's expected with external packages, but not with app specific code.

The stuff in node_module is app specific code. The moment you npm install it stops being external, npm just dumps remote code into a local folder. node_modules resolution is not bound to npm or any external registry. They are completely different constructs.

@grundmanise
Copy link

@Rich-Harris you are actually starting to expand my mental model 🔮

@necauqua
Copy link

Well, the existing mental model is that the src folder contains the files you wrote and you manage.

I don't know why it suddenly needs to be changed to the 'well yeah but also there is a node_modules folder - yup, same name as the one that you already learned to be afraid of and also never ever checking it in VCS (that one you also have one level above btw), and so this one is managed by something else, so ignore it plz'.

This whole argument above about the evolution of node_modules folder makes little sense - so you are using the module resolution designed for something else as a hack, and right after that you say we don't want stuff to be hacky or use aliases or whatever, we want to change the mental model (to what?).

So weird and pointless, for me at least.

Also there exists a concept of a generated sources folder, it even has a separate icon for it in IntelliJ, why won't you use something like that, idk.

Okay, I wanted to end the message here, but I've read the whole thing again and here is what I understood:
So the main problem is that npm may nuke extra things added to main node_modules folder (that as you point out can be used for anything that you want to be resolved).
And the npm also has this built-in hack of also scanning the src/node_modules, right?
And you just use it, because aliases are bad for reasons, and there is not much else you can do.

So anyway I have to conclude that this all is really weird and perhaps I would wait/ask/contribute for some tools for autogenerated source code in the npm itself.
Like so you can put it somewhere else (because putting it in src is dumb and I cannot imagine you proving to me that it is not) or maybe so it wont get nuked from the big node_modules, idk.

Anyway, as I've said some time before above, there is a switch for me to get this sweet-sweet non-polluted src folder at expence of having to rebuild the project (I guess?) after npm install'ing something, so I am writing all of this just for the debate mostly. And because we disagree, of course, thats how humans work.

@pngwn
Copy link
Member

pngwn commented Jun 19, 2020

npm also has this built-in hack

This has absolutely nothing to do with npm. The node_modules resolution algorithm is not related to npm. As Rich stated npm came after node modules was implemented and npm simply leverages it. And this is not a hack in the slightest, it was explicitly designed this way for a reason. You might not like the design, but you need to take that up with node.

@Rich-Harris
Copy link
Member Author

the existing mental model is that the src folder contains the files you wrote and you manage.

Again, as written in #551 (comment):

The interesting distinction isn't between stuff-i-wrote and stuff-i-didn't-write, it's between stuff-that's-specific-to-this-app-or-library and stuff-that-comes-from-elsewhere.

It's not written in stone anywhere that src can only contain code written a character at a time by your own hand (and if it is, better not let Prettier touch it!).

Is src/node_modules a perfect mechanism along all dimensions, including the aesthetic? No. Is it good enough? Yes. Is it better than all the others? Absolutely.

@necauqua
Copy link

nothing to do with npm

Okay, not npm but node, or webpack or who does the actual resolution - resolving stuff from src/node_modules just looks like a historic quirck of some sort.

I get that this is the best you can do maybe at this point and you cannot do anything better, but what I don't get - is you defending this obviously wacky solution like it is the best thing you've ever done and that this is how it should be done.

not written in stone anywhere

Well, yeah, but is seem most reasonable, nobody nowhere ever puts generated code just right into src (point me to such things if you know them, I am interested) and your point about formatters is pretty far-fetched since you are trying to find fault in my point with your 'every character written by you' that I've never said.

From googling why the src/node_modules exist in the first place I found out that it is too weird for most people (what a surprise) and instead of your 'I'm gonna change how everyone thinks' mindset they chose other solutions.

Okay, ending the argument, now the actual question since I know less than you anyway (yeah this statement might lead to you not changing your mind because meritocracy, but you wont change it anyway so I can remind you of that fact because humans like that):

Is npm nuking it out from the big node_modules even bad? Wouldn't the thing just be regenerated on next build/run? I feel like I am missing a point here (ok maybe regeneration is time-consuming for big projects idk).

@jakobrosenberg
Copy link

I'm not proposing one method over the other, just addressing a biproduct of the mental model that might have gone unnoticed.

We assume from conventions, that a src folder is a safe haven for our code, while dist, build, bundle etc will be overwritten. The mental model is a simple "from" and "to" concept, that cleanly separates the code you manage from the program managed code.

  • It's extremely easy to reason about and decreases cognitive load
  • It's good practice
  • It's safe
  • It's good DX
  • It's an established convention

Good conventions lead to good DX.


On a side note. I don't get the nuke reasoning. Isn't the folder generated at runtime and won't it be regenerated if it's missing?

@Rich-Harris
Copy link
Member Author

The mental model is a simple "from" and "to" concept, that cleanly separates the code you manage from the program managed code.

This might be the mental model that a lot of people currently have, but it's simplistic at best — the boundary is more porous than that:

  • TypeScript will, by default, write .js files next to the .ts files in src when it's compiling them
  • Programs like Prettier overwrite your source files
  • It's not that uncommon to have script-generated files in src. I do it all the time in my day job. An earlier version of Svelte gave you the option between importing helpers like element from a shared library or inlining them into the output (intended for standalone components), and that relied on a file called internal_exports.ts being generated (and gitignored) so that it could be imported into an adjacent module — sure, it could have been put in generated/internal_exports.ts rather than src/compiler/compile/internal_exports.ts, but then it would have been much less clear where it belonged in the overall architecture

Isn't the folder generated at runtime and won't it be regenerated if it's missing?

It is regenerated when you change the structure of your application (by renaming files in routes, or adding/deleting them). If you do npm install thingamajig then import thingamajig into an existing file, it will get nuked but not regenerated, unless you then a) deliberately trigger a regeneration somehow or b) stop and restart the process. In the meantime, you have a rather confusing error. That's a shitty developer experience.


All this is moot though, because no-one has come up with any workable alternatives. (Aliases are a terrible, horrible, no good, very bad workaround, and we will not be using them.) Please restrict any further comments in this thread to sensible and considered alternatives rather than griping about aesthetics, or we'll have to lock this thread, because it's not going anywhere.

@jakobrosenberg
Copy link

Again, it makes no difference to me. I'm just here to provide feedback as long as it's helpful. 🙂❤

A couple of alternatives

  1. You could proxy the generated folder through the Sapper package.
  2. You could regenerate node_modules/sapper on npm run dev.

Both may come with drawbacks. Especially option 1 might clash with how tools like yarn and pnpm resolve modules. Someone smarter than me might see a way to do either without drawbacks.


As for the mental model, I think it's important to differentiate between

  • formatting vs overwriting. Auto formatting is irrelevant to whether I can safely work with the files.
  • script generated files vs program managed files. How the code originated is irrelevant to whether I can safely work with the files.
  • file mappings vs a singular build. I'm not a fan of a program managing either in the src folder, but I can see a reason for the former.

I know the last half isn't constructive in terms of an alternative solution, but it could help when weighing the pros and cons.

@pngwn
Copy link
Member

pngwn commented Jun 19, 2020

I think the issue with all of the alternatives that people suggest is that they require additional complexity and maintenance. The default behaviour might be upsetting to some due to how npm-centric the javascript ecosystem has become, but it is default, specced behaviour and requires no additional work to function.

It is unlikely to change regardless, we understand people's concerns but are happy with the trade-offs.

@moon-bits
Copy link

@Rich-Harris, just to put the atmosphere of the discussion into the right setting. I'm thankful for Svelte/Sapper. It seems that this discussion is just based on an imbalance of knowledge as @necauqua pointed out philosophically using meritocracy.

Yet, it is important to state the drawbacks and advantages of people's suggestions of their solution to this issue, instead of arguing like a prophet that you are here by changing our mental model...We are developers/SEs, not some backwoods. We need the understanding why the provided solution is not feasible.

A couple of alternatives

  1. You could proxy the generated folder through the Sapper package.
  2. You could regenerate node_modules/sapper on npm run dev.

Regarding 1., I'm not sure what you mean by that exactly, @jakobrosenberg

Regarding 2., I think @Rich-Harris stated the drawback of this alternative and I think that I understood his point of view now. Please confirm/correct me if I'm right/wrong, @Rich-Harris:

By putting "dev": "sapper dev --output ./node_modules/@sapper" within my package.json everything is working fine until I put somewhere an import statement. Then I would have to kill my running npm run dev and start it again otherwise it will open a black hole in the universe and I'm starting to see strange error messages?

It is regenerated when you change the structure of your application (by renaming files in routes, or adding/deleting them). If you do npm install thingamajig then import thingamajig into an existing file, it will get nuked but not regenerated, unless you then a) deliberately trigger a regeneration somehow or b) stop and restart the process. In the meantime, you have a rather confusing error. That's a shitty developer experience.

Thanks again to everyone that works on Svelte/Sapper! Yet, I think it is important to have a clear understanding of your decision for others that get surprised by a node_modules in their src/ tree. They need to be able reason about that instead of just stamping it as a shitty solution.

@pngwn
Copy link
Member

pngwn commented Jun 20, 2020

instead of arguing like a prophet that you are here by changing our mental model...We are developers/SEs, not some backwoods

I don't think that was Rich's intention. Svelte has a long history of going against convention because we don't think the current ways of working are particularly fruitful, this is another case of that. There is lots in Svelte 3 that go against common practice and that people weren't sure about at first but quickly got used to as they saw the benefits. This is a similar situation, in some ways it is less controversial because we are actually removing a bunch of work and complexity in taking the current approach, and deferring to standard behaviours.

@jakobrosenberg
Copy link

jakobrosenberg commented Jun 20, 2020

Regarding 1., I'm not sure what you mean by that exactly, @jakobrosenberg

@moon-bits, if you know the location of the generated files, you can import them from within a package and from there export them.

a-package

export * from './path/to/generated/file'

user code

import {foo, bar} from 'a-package'

A third option C) would be to import Sapper at the root level of the app and sharing it through Svelte's setContext. The extra set/getContext boilerplate could be abstracted into a small helper.

I've used variations of both.


@pngwn I fully agree with your critical approach to conventions. Even if I think the costs outweighs the gains in this specific case.

@moon-bits
Copy link

moon-bits commented Jun 20, 2020

@jakobrosenberg, then why not simply define some webpack alias?
As far as I can see, Sapper makes already use of webpack. So instead of (quoting @Rich-Harris):

import { app } from '../../../../../../__sapper__/client.js';

We could define a webpack alias, called 'sapper-internal' that makes it easier to import the app object.

import { app } from 'sapper-internal/__sapper__/client.js';

This issue is also visible when trying to import a component outside the components/ folder. In other frameworks there is an alias for the src/ root where you just import

import MyComponent from '@/components/my-component';

instead of (current approach)

import MyComponent from '../../components/my-component.svelte';

@pngwn When you say that it increases complexity and maintenance, are you referring to that? AFAIK defining webpack aliases is pretty simple:

//...
resolve: {
    alias: {
      '$@': path.resolve(__dirname, 'src/'),
    }
  }

@pngwn
Copy link
Member

pngwn commented Jun 20, 2020

Sapper supports two bundlers. I'm personally of the opinion that aliases are a poor solution, in order to work out where things actually live, you have to go and read through a bundler config which is often not a simple task.

@moon-bits
Copy link

I'm personally of the opinion that aliases are a poor solution, in order to work out where things actually live, you have to go and read through a bundler config which is often not a simple task.

Hmm... I'm more confused right now 😕

  • node_modules/ is being hidden by most IDEs - so it makes it even more difficult to figure out where stuff lives - not mentioning to even understand what that stuff is doing in a src/ folder
  • by creating a node_modules/ folder within the src/ folder you confuse most developers, since it is an unorthodox approach
  • having everything in the __sapper__/ folder would align more to your argument, since it's the only place where all Sapper related stuff would live in
  • defining 4 lines per bundlers is really not worth a discussion of +50 comments 😄

@pngwn
Copy link
Member

pngwn commented Jun 20, 2020

Aliases are arbitrary, node modules resolution isn't.

defining 4 lines per bundlers is really not worth a discussion of +50 comments.

We aren't adding any lines, making it even more absurd.

@jakobrosenberg
Copy link

then why not simply define some webpack alias?
@moon-bits

Aliases are a terrible, horrible, no good, very bad workaround, and we will not be using them.
@Rich-Harris

That's why. 😋

@pngwn random aliases are bad, yes. Exceptions for root or a framework... I don't know. / is a common "alias" for the root and if that isn't available, @ or root seems like an okay alternative. Not saying that's a solution for Sapper, just that aliases don't have to be confusing.

@benmccann
Copy link
Member

This seems a bit like bike shedding to me. I'm not sure what practical problems this has caused anyone. If we all have time to spend on this issue then surely we also have time to spend on all the real issues like adding TypeScript support, preloading assets, SPA mode, etc., right? 😄

@Rich-Harris
Copy link
Member Author

Just going to spell this out once and for all:

Why aliases are a terrible idea and should never have been invented

In Sapper's specific case they're particularly bad, because you can't just define them in one place — you need to define them for each of the three configs (server, client, service worker), and since we maintain a project template for both webpack and Rollup, multiply that by two.

But they're not just a bad idea in the context of Sapper, they're a bad idea period. Add aliases to your webpack config, sure, but then your editor doesn't know about them, so you can't take advantage of handy things like auto imports and whatnot when writing .js files (and .svelte files, as editor extensions improve). Nor does your linter, or your typechecker if you're using TypeScript. So you need to figure out how to add those aliases too. Who knows how that extra maintenance burden will manifest itself in future if new tools like Snowpack and Vite become popular? All this bullshit configuration, multiplying itself and forcing its way into new tooling, creating technical debt all over the place. Just madness. And totally unnecessary.


Proxying is an interesting notion, but it's restricted to a single module. So you need to create separate proxies for these...

import * as sapper from '@sapper/server';
import * as app from '@sapper/app';
import { timestamp, files, shell, routes } from '@sapper/service-worker';

...and more for any others in future, not to mention any userland additions (I'll typically have folders like src/node_modules/@components and src/node_modules/@utils etc). On top of that, it's an extra layer of magic — it seems like you're importing an actual module, but you're not — you're essentially importing a reified version of your app as defined by the file tree inside routes, which obviously doesn't belong in a place that's normally reserved for third party modules. Oh, and you still have to put those files somewhere. So it really brings no benefits other than avoiding the momentary 'huh, interesting, why is there a src/node_modules folder?!', but a variety of disadvantages.


But Ben is right! This discussion has long since outlived its usefulness. No-one has come up with any good alternatives, so I'm going to lock this thread.

@sveltejs sveltejs locked as resolved and limited conversation to collaborators Jun 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests