Multiple context=module Scripts
#27
Conversation
|
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 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 @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 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>
|
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:
|
|
You could implement the actual proposal in a preprocessor unless there is more to it than the RFC communicates. Write This is why I asked these questions, which haven't been answered.
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.
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 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 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) |
With
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.
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)
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 |
|
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. |
Not really, nextjs doesn't handle this. |
This comment has been minimized.
This comment has been minimized.
|
As a reminder to all, the purpose of this RFC to assemble the module exports dynamically based on the compiler's current As of now, we only have 1 There is nothing runtime-based for this RFC. 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 };
}
Also, as mentioned, modifying the
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 |
|
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 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.
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?
Compiler macros and solving a niche solution for a particular case are not in the same category. Implementing a
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. 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 Yes, I personally have run into this and wished I had it. Does it affect only myself? No. Does it still feel wrong? Yes. 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
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 (#27 (comment)). 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:
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:
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:
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.
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 @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 ( @ajbouh I don't think Worker/ServiceWorker contexts should be incorporated. While that could be cool, the "issue" here is that
@ajbouh I've mentioned it in this answer already, but it's primarily about inconsistent/mismatched |
|
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 |
|
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: |
|
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 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 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 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
...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 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 |
|
All of the component source code needs to be present in the AST in my opinion, or the AST isn't a true representation of the source code. We don't necessarily need to incur the cost of parsing the contents with acorn at that point if we move some things around but I feel like we should at least have some idea of what the entire source component looks like in the AST. |
|
This moves us on to another question, what about naming conflicts etc? Does svelte resolve them or some other tool? Or is that on the user as it is now? |
|
I feel like the env specific ones would extend the shared module. I don't think conflicts should be tolerated really so I don't envisage any special handling of merging exports etc. |
This is what I hinted at in my thought on how to get this working for the IDE, too. There are name clashes which you would want ( |
Rendered