rfcs
rfcs copied to clipboard
Multiple `context=module` Scripts
redacted
I'm a little confused about what the RFC is requesting comment on. Is this proposal for the svelte parser to support arbitrary context scripts, or is it to add two additional context module scripts which svelte would then include/ exclude depending on generate: 'ssr' | 'dom'. It looks like the latter.
I guess my main thoughts here are around how common this problem is (in general not for you personally) and if it is particularly niche then how difficult is this to implement in a preprocessor.
My other concern is, why limit this to module scripts only and not extend it to instance scripts? But then we have the issue of ballooning complexity, we would be up to 6 script tags if we did that.
I wonder if we would be sacrificing ergonomics here by introducing lots of script contexts. I don't agree that teaching this is as simple as adding a few notes to the docs, we need to be able to clearly communicate which script you use and when to users, simply providing more entries to the docs doesn't do that. Having too many options is definitely confusing, especially for newcomers, and we would need to take care to avoid a situation where user had too many options and didn't know what to do. One of the main design principles of Svelte is to keep things simple as much as possible and I'm not convinced this is in line with that.
I'm very wary of this proposal because as far as I can work out this functionality is actually already possible, this would just improve the ergonomics with an official API that, in my view, has the potential to generate confusion.
@sw-yx I think I agree – but for now, at least that has svelte-preprocessor support (or plain svelte.preprocess). This RFC has no current workarounds. Let's also try to keep the conversation limited to this RFC -- I'm guilty of adding inspired-but-unrelated ideas to RFCs too 😅
@pngwn Any new feature has to go through an RFC. In my mind, this is a pretty certain need and there's very little to bikeshed (syntax only). As you've said, I'm suggesting the latter. And I'm only suggesting context=module exactly for the reasons you've stated. While there's no technical reason as to why you can't have unlimited conditional scripts, I'm only suggesting two as they're the two that's required/blatantly missing once you run into the use case. Not going to address the ballooned-complexity scenario(s) because that's not what's put forth here.
This functionality is not possible now with Svelte. It's only made doable by configuring Rollup/webpack aliases, which, at a basic level, allows for dependency aliases, but in order to define different handlers of your own, you have to hoist those functions outside and away from of the component and then alias that path depending on output mode.
<script context="module">
// change dependency alias
// (dom) `httpie/fetch` vs (ssr) `httpie/node`
import { send } from 'httpie';
</script>
<script context="module">
// change local path alias
// (dom) `./my-client-preload.js` vs (ssr) `./my-server-preload.js`
export { preload } from './preload';
</script>
☝️ shitty ergonomics
In my mind, this is a pretty certain need and there's very little to bikeshed
I honestly couldn't disagree more.
I've spent a lot of energy dealing with very similar challenges. I don't think svelte is the right layer to solve this in.
In each case so far I've solved it 100% in sapper without any changes to svelte.
When considering the impact of this change, most of the benefit happens for very small projects with only a few components. Beyond this you end up needing so much repetitive boilerplate that I don't think the ergonomics are clearly better.
Can you say more about why you don't think there's a workaround that's possible without modifying svelte?
Some questions I have:
- How does Next handle this?
- I'm not sure that this example really demonstrates the need to me. Is the overhead of an http call to localhost so high that it warrants this new feature?
- Can you use Rollup's replace plugin like Sapper does? (pseudo code below. the syntax is probably a bit off)
<script context="module" lang="ts">
export async function preload() {
const req = this.req;
if (!process.browser) {
// Bring database dependencies for server
const sql = await import('postgres');
let loaded = {};
loaded.slug = req.params.slug;
const rows = await sql<IArticle>`
select * from articles
where slug = ${loaded.slug}
and deleted_at is null
limit 1
`;
if (rows.length) loaded.data = rows[0];
return loaded;
}
loaded.slug = req.params.slug;
return fetch(`.../${loaded.slug}`).then(r => r.json()).then(obj => {
loaded.data = obj;
});
}
</script>
You could implement the actual proposal in a preprocessor unless there is more to it than the RFC communicates.
Write module, module:dom and module:ssr scripts in your component. In a template preprocessor find all 3. if process.browser merge module and module:dom scripts, otherwise merge module and module:ssr scripts. You will also need a little logic to ensure things work when some/ all scripts do not exist but that isn't complex.
This is why I asked these questions, which haven't been answered.
I guess my main thoughts here are around how common this problem is (in general not for you personally) and if it is particularly niche then how difficult is this to implement in a preprocessor.
I personally feel this API paves the way for DOM/SSR versions of both scripts, so the 'ballooning complexity' comment is still relevant, as Luke said there is no reason it shouldn't also be there if this is. This makes it highly relevant because this feature would essentially state 'we want script variants'.
I also think this use case is relatively niche, it isn't incredibly common to have your datastore right there with your node server in my experience, most users are using API request to access that data anyway. I'm sure when you run into the issue it is frustrating but it is also resolvable by various means.
I'm commenting 100% from a user perspective.
this would just improve the ergonomics with an official API that, in my view, has the potential to generate confusion.
I don't share the assumption about confusion. There's great many things a newcomer likely skips when learning Svelte - for example probably the entire "Compile time" section at the end of docs. Or at least I did :) If I understood correctly, the proposed output flags would be entirely optional, so they wouldn't need that much attention in the docs, they would just have to be there when needed. They are also quite self-explanatory, which is why I think they'd fit in easily with minimal documentation changes.
I guess my main thoughts here are around how common this problem is
I just started building an app with Svelte utilizing SSR in Cloudflare Workers. This feature would have come in handy when I realized that our "server" environment's (the worker) implementation of certain APIs differ slightly from the browser. Instead of polluting our code with if-elses and some env-specific globals, this seems like it would have been a clean and readable solution to differentiate between the two.
That said, I'm only starting out with SSR and Svelte, so my view is based on very little hands-on experience. (Though based on that brief period of hacking with SSR and Svelte, I do understand @lukeed 's motivation for this)
- How does Next handle this?
With export getServerSideProps and export getStaticProps => next docs
- I'm not sure that this example really demonstrates the need to me. Is the overhead of an http call to localhost so high that it warrants this new feature?
For our team, it turns out that 90% of the preloads are simple http calls to equivalent APIs. I'm used to and I like the mental overhead to express data dependencies in APIs, but it seems like more work for limited benefits. It spreads out related logic over different directories. As far as I have seen, it's also a typical footgun for beginners.
- Can you use Rollup's replace plugin like Sapper does? (pseudo code below. the syntax is probably a bit off)
Last time I checked, Rollup would still take a lot of time to bundle the dependency only to remove it afterwards in production. Which may be good enough? (not sure how next.js improves over that)
I also think this use case is relatively niche, it isn't incredibly common to have your datastore right there with your node server in my experience, most users are using API request to access that data anyway.
Even in the case of using APIs, you typically want to keep your API credentials secret, so also this case could benefit from reduced complexity.
So my proposal is to keep only 1 context=module, add preloadServer() and optimize client bundle compilation in that case by somehow removing unused imports/variables.
Also relevant is how things should work in the service worker. DOM APIs aren't actually present in the service worker so if this is the hammer we use for solving the problem of "different implementations for different execution contexts", we need to introduce enough surface area to ensure that it solves the issue to a reasonable extent.
@thgh @benmccann
How does Next handle this?
With export getServerSideProps and export getStaticProps
Not really, nextjs doesn't handle this. getServerSideProps always executes on the server. There is no concept of isomorphic fetching there. When you perform a client-side navigation to a route that uses getServerSideProps it makes a call to an endpoint that runs your getServerSideProps function and returns the data. If you are making an API request from your getServerSideProps function then this introduces an unnecessary network waterfall.
For me the question is: what should render_ssr(component, otions) src/compiler/compile/index.ts#L96 return ?
Currently it returns const {js, css} js is obviously some javascript which is intended to run on the server. However, we have very limited control on this js code. It's sole purpose is to render the component and it returns const {html, head, css}.
In the current state, we can not use any server features in render_ssr despite it's purpose to run on the server.
I don't know the point of view of the core team on the topic. But maybe they want to keep svelte as a pure UI generator limited to {html, head, css}. In that case, I assume everything related to SSR should be handled in some external tools like sapper.
My workaround
What I plan doing is the following:
MyComponent.svelte
<script context="module">
export const initialData = {
init: "init with this data",
async: {
url: "https://host/api"
}
};
</script>
in my custom build script on the server
require('svelte/register');
const Comp = require('./src/MyComponent.svelte');
let DATA;
if(Comp.initialData){
// do sometthing with initialData like fetching from some API
// parse api results
// store everything in DATA
DATA = {...}
}
const { html, css, head } = Comp.render(DATA);
// etc...
I decided to go with my own build script since sapper takes a whole different approach on serve side rendering SSR and static site generation SSG.
As a reminder to all, the purpose of this RFC to assemble the module exports dynamically based on the compiler's current options.generate target.
As of now, we only have 1 context=module instance which defines custom exports for the components for all outputs.
This RFC offers a non-breaking change, allowing most users to keep a single context=module script if it serves their needs.
There is nothing runtime-based for this RFC.
There is nothing specific to Sapper, Routify, Next.js, etc in this RFC.
The only tie-in is that by allowing Svelte to construct module contents dynamics, tools like Sapper and Routify have the option to do things differently at runtime, simply because their components' exports were able to be constructed different for the different environments.
At the bottom of the RFC is this pseudo-code snippet that sums it up nicely, IMO:
let module = {
default: Component,
...exports // context=module
};
if (has_context('dom') && options.generate === 'dom') {
module = { ...module, ...exports_dom };
} else if (has_context('ssr') && options.generate === 'ssr') {
module = { ...module, ...exports_ssr };
}
That's it.
Also, as mentioned, modifying the context=module contents can be done through juggling extra files (and importing them) and/or dealing with Rollup/webpack aliases, but that's:
- heavy reliance on Rollup/webpack
- breaks the Single-File-Component model
- litters component logic across up-to-3 locations (component, extra file(s) for env-based logic, rollup/webpack config)
Given that the Svelte compiler's current role is (a) to build the component output based on env (SSR vs DOM) and (b) to assemble the module export list; we're missing (c) assemble the module export list based on env (SSR vs DOM).
But as Svelte has separate DOM and SSR builds, you can replace the module context script however you wish before they reach svelte and the contents will be respected. Svelte could make it a bit easier by exposing the generate option during preprocess, but this is solvable in a preprocessor, so aliases or custom resolution rules are not the only workaround.
My question is, is this a common enough problem that Svelte should handle it directly, or would a preprocessor be sufficient. I'm unconvinced that this is a common problem as, as far as I can recall, it has been mentioned twice in all the time that I've been supporting people using Svelte and both were recent occurrences.
I'm saying that it's a lack/inconsistency that Svelte does not already do this. It's not going to be a "common" problem, mostly because (a) most people who do anything with Svelte SSR are only doing it through a framework in the first place (b) it's not been offered before, so collecting a "this is not common" tally is biased and makes no sense.
The preprocessor argument is moot IMO because you can do just about anything with preprocessors. For example, with enough effort, you could implement reactive assignments with a preprocessor – but that doesn't mean it was a bad idea to live inside Svelte directly.
The point is that the compile step does two things (template & module preparation), but it only allows one of them to be output/environment aware. Ideologically, this is missing.
I don't know what bizarre rhetoric you are employing here in order to make your case but I'm not finding it particularly endearing. You are completely dismissing every question without answering in any meaningful way. It just sounds like you have made up your mind and refuse to engage with any of the questions.
It's not going to be a "common" problem, mostly because (a) most people who do anything with Svelte SSR are only doing it through a framework in the first place (b) it's not been offered before, so collecting a "this is not common" tally is biased and makes no sense.
I asked it if the core problem, i.e. needing different output on client and server, was common not if people trying to solve it was common. Whether or not svelte supports it doesn't make a problem go away. If this is such an insignificant real-world problem that people can simply ignore it because Svelte doesn't support it, then that doesn't further convince me. If it were a common problem and this were a common need then there would be solutions or more requests for this feature regardless of framework use or direct svelte support. Asking if this is a common problem makes perfect sense, otherwise how can we work out whether the feature is at all useful?
The preprocessor argument is moot IMO because you can do just about anything with preprocessors. For example, with enough effort, you could implement reactive assignments with a preprocessor – but that doesn't mean it was a bad idea to live inside Svelte directly.
Compiler macros and solving a niche solution for a particular case are not in the same category. Implementing a context:dom is a more ergonomic and less constrained way of using process.browser or aliasing, not changing the semantics of the framework. It isn't 'moot' because it is a viable solution. You used a bundler-based solution as an argument in favour of this due to ergonomics, so addressing this in a preprocessor isn't an absurd idea. Typescript support is a hugely desirable feature, for example, but we didn't implement that in core either.
The point is that the compile step does two things (template & module preparation), but it only allows one of them to be output/environment aware. Ideologically, this is missing.
Again these aren't the same thing, environment aware user code is not supported in any context. Svelte's output changes but that is all, there is no Svelte provided hook to generate different user-code for different targets. This would be a completely new change and has no parallel in Svelte right now.
To be honest, it feels like you're trying to argue for the sake of it, and not in a fun way.
In the beginning I wasn't even sure if you had read the full RFC. Otherwise I don't think there'd be any question re: "I think the latter". You then followed up by taking the RFC to hypothetical places that were not included in the RFC at all, and used that complex, messy, "ballooned complexity" scenario as a point of concern to make your point. I'm only suggesting two additional, opt-in scripts (not 12, not 6, not instance scripts) so that the module's exported interface can be modified. Again, module.default (the results of the component compilation) are the only exported member that can be tied to the environment. The rest of the module may be stuck/expecting some other state.
Can this be done other ways? Of course – and currently, it has to be. That's the essence of what I've been saying – and why I've not been addressing it directly.
We can call this a "nice to have" more than a "must have" if that'll make you happy. The fact is that this does have to be worked around and considered when using a non-off-the-shelf framework like Sapper. But even then, Sapper itself has/had to work around this by carefully constructing a preload(req) interface and by directing users to a (shimmed) this.fetch reliance. But, as we've seen, there are requests to get out of that – some want to customize this.fetch and some want to hook in database queries. Among other, perfectly plausible solutions, solving it at this level allows for all those requests and so much more.
Yes, I personally have run into this and wished I had it. Does it affect only myself? No. Does it still feel wrong? Yes. If you revisit my 1-3 list, that's why the current solution(s) feel so non-Svelte-like.
TypeScript support is an obvious add-on. Not only is it reliance on an external tool/compiler, but Svelte is not claiming responsibility for the syntax or features that TypeScript brings.
However, Svelte has volunteered itself responsible for its components' modules' interfaces. This happened the moment script[context=module] became a thing. And because it's now responsible – and has an option-based module.default content – it should also allow for the rest of module to be tied to that option's value.
It just sounds like you have made up your mind
To open an RFC, you'd (hopefully) have thought through what you're proposing before actually proposing it. So yes, of course, I think this is the right approach. Otherwise I wouldn't bother. I'm open to feedback and suggestions, but you're not suggesting anything – you're just poo-poo'ing it, which is fine and you're perfectly entitled to do so. But I'm not really here to state the obvious – there are obviously workarounds.. they're not good, that's why this exists.
The most appealing alternative/suggestion so far was @benmccann's dynamic import + replace solution (https://github.com/sveltejs/rfcs/pull/27#issuecomment-683888282). My only gripe with that is that it's still too much reliance on Rollup/webpack. If you were to compile the component file with Svelte alone, it'd break.
I don’t think I was poo pooing. I had some concerns and asked a few questions, one of which I had to repeat 3 times before it got a response.
I mentioned the preprocessor because it isn’t just a way to solve the problem but is actually a way the entire thing can be implemented in userland with all of the good ergonomics and few trade offs. Which is why I wanted to discuss it.
I don’t think comments like this are helpful, it sets the tone pretty clearly:
Any new feature has to go through an RFC. In my mind, this is a pretty certain need and there's very little to bikeshed (syntax only).
Maybe I read this wrong but this just reads like “I have decided we need this but I need to go through the motions to get it in”.
This was the full sentence that you snipped:
It just sounds like you have made up your mind and refuse to engage with any of the questions.
That second part of the sentence is pretty important. I would indeed expect you to believe this feature is worthy of addition, I would also expect some engagement with the RFC process, questions and concerns around the feature.
Oh, sorry. I can see that. I was responding to two different different things, and I probably should have quoted or at least broken up my paragraph:
I'm a little confused about what the RFC is requesting comment on.
"Any new feature has to go through an RFC"
I guess my main thoughts here are around how common this problem is (in general not for you personally)
"In my mind, this is a pretty certain need"
"and there's very little to bikeshed" Could have done much better with this, but I was trying to reel back the "too many scripts" hypothetical since that's not what I was putting forth. Sorry
That second part of the sentence is pretty important.
Yes, but I was considering this an obvious statement so I didn't address it. I think there's very little you can't do with preprocessors, so I didn't know how that was helping. It felt like it was added just to further with the "I think this is only your problem, here's how you can one-off it" vibe I was getting.
would also expect some engagement with the RFC process, questions and concerns around the feature
Yes, of course. I could do/have done better. Yesterday wasn't the best (doesn't excuse it) and I was carrying some of those (mis)perceptions into today. Sorry~
@igorovic @thgh I'm not proposing any runtime hooks. This RFC is only about aligning the compiled module's export list to match that of the component's (which resides at module.default).
@thgh @benmccann Last I checked, @pngwn is correct. Their solution is to look at conditionally-defined methods. That's still a runtime solution that can only hope to be fully tree-shaken away during through webpack + terser, if at all. Svelte (directly) is unique in that it already controls the module interface. What I'm trying to do is point out that the module interface can easily be inconsistent once it leaves Svelte's hands (module.default has SSR content & everything else (module.*) is assuming DOM usage; or vice versa). /cc @ajbouh
@ajbouh I don't think Worker/ServiceWorker contexts should be incorporated. While that could be cool, the "issue" here is that generate: 'ssr' will output a component module whose module.default is for SSR, whereas everything else module.preload (as used in the RFC example) will be expecting DOM usage (and so also carrying DOM APIs). I think there should only be two options: module:dom and module:ssr to match the generate options.
Can you say more about why you don't think there's a workaround that's possible without modifying svelte?
@ajbouh I've mentioned it in this answer already, but it's primarily about inconsistent/mismatched module exports. A secondary reason is that having it at the Svelte-level can aide Sapper-like frameworks so that they have less to setup themselves or less to workaround. Having built one of these (and having helped clients setup custom SSR solutions), the inconsistent module contents proves to be an obstacle and limiting factor for what you can put inside context=module. As @pngwn said, this can be resolved at the preprocessor level (or there can be framework overhead/polyfills) to allow for flexible context=module scripts (or any), but that's all still done because Svelte is spitting out modules that aren't compatible with themselves.
Hey, sorry for going offtopic, just tried to add context to the discussion 😆 after re-reading everything: I think this feature makes sense. I lean towards option 2 and would consider <script generate="ssr" as to align with the svelte api.
@thgh Thanks! My hesitation with Option 2 echoes one of @pngwn's concerns: It makes it seem like you can attach generate="ssr" or output="ssr" to any script tag and it'd be reserved for that mode. I don't think that'd end well if it were to be the case
It doesn't have to be implemented right away, but the question will come up probably. When bundling a client-side library that isn't server ready, it could be a way to render a placeholder. I'm thinking of graphs for example.
I just want to mention that there is a related discussion here, in case you haven't seen it: https://github.com/sveltejs/svelte/issues/4741
I'm trying to figure out how scoping would work with this, and I'm not entirely sure. Currently, the template can access things in the instance or module scripts, and the instance script can access stuff in the module script. What would happen with this? I'm thinking about this both from a functionality perspective and from an ESLint no-undef/no-unused perspective.
Relatedly, we'd also need to decide what the AST looks like.
@Conduitry This why I think there needs to be an instance version of this too. Otherwise it feels like we'd just be introducing a footgun. Im curious about this this would work from a typescript point of view too @dummdidumm.
Right now the module script is pasted at the root of the generated file, so this:
<script context="module" lang="ts">
let loaded = {};
export function preload(req: IRequest) {
// ...preload
}
</script>
<script lang="ts">
// ...instance script code
</script>
<h1>html stuff</h1>
Becomes this:
///<reference types="svelte" />
<></>;
let loaded = {};
export function preload(req: IRequest) {
// ...preload
}
;<></>;function render() {
// ...instance script code
;
() => (<>
<h1>html stuff</h1>
</>);
return { props: {}, slots: {}, getters: {}, events: {} }
}
// ...
As you can see, the module script content is at the root while the instance script is inside the render function.
If there are now multiple module scripts, one of them would have to be the "lead", so only one of them is at the root and the others are within functions similar to the render function. Which one would that be? Also, in the example of the RFC loaded is used in both the SSR and the client module version, which makes sense. But how would the type checker ensure that you don't have a typo in one of the loaded variables?
A solution might be to transform this so both the ssr and the client module script is put into a function and all top level declarations are returned. Something like this:
///<reference types="svelte" />
<></>;
function ssr() {
let loaded = {};
// ...
return {loaded};
}
function client() {
let loadeed = {}; // <- oops, typo
// ...
return {loadeed};
}
;<></>;function render() {
const {loaded, loadeed} = unionReturnTypeOf(ssr, client);
// <-- error: loaded does not exist on type {loaded: {}} | {loadeed: {}}
// Problem left: This error is inside generated code, where to put it?
// ...instance script code
;
() => (<>
<h1>html stuff</h1>
</>);
return { props: {}, slots: {}, getters: {}, events: {} }
}
// ...
This would of course only solve the problem from the intellisense point of view, AST -> tbd. Maybe different solutions also arise when a solution to the AST problem is found.
I'm thinking through how this would play out in the context of Sapper over on #31. tl;dr is that it would allow @thgh's team (and me, and anyone else who writes a bunch of very predictable preload functions)...
For our team, it turns out that 90% of the preloads are simple http calls to equivalent APIs
...to write a bunch less code.
I've also included (here) an example of how this could be achieved with a preprocessor. I agree with @lukeed that if something is sufficiently important it should exist in core, with properly defined and documented semantics, though it's as yet unclear whether the use case outlined in #31 meets that threshold. Preprocessors do, at least, give us a workable way to try out these ideas in a relatively low-risk way.
My thought was to only parse the script contents on matching ssr/dom mode. At that point it's the same AST and scoping rules as a singular context=module.
And then on matching block, its contents are pasted at the top/root of the output file.
An unanswered question that I share with @dummdidumm is what should the output order be when all 3 scripts are defined:
<script context="module">
export const shared = true
</script>
<script context="module:dom">
export const dom = true
</script>
<script context="module:ssr">
export const ssr = true
</script>
If building for ssr, should the output contain shared then ssr, or vice versa?
If there are export conflicts, it'd make sense to have the env-specific block take precedence, but that would still implicate the "what order" question and would require some internal tree-shaking before even printing the context=module to root.
If the answer is/needs to be "only one script block per mode" (aka, can't have 3 defined) I think I'd be fine with that. It'd simplify a lot on the parsing and compile steps, since all it changes is which script becomes the context=module stand-in. The other script, if present, gets treated as if it didn't exist and does t make it into the output at all.