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

Phase 2-1: safely instantiating SourceText in Main ModuleMap #208

Closed
SMotaal opened this issue Oct 24, 2018 · 39 comments
Closed

Phase 2-1: safely instantiating SourceText in Main ModuleMap #208

SMotaal opened this issue Oct 24, 2018 · 39 comments

Comments

@SMotaal
Copy link

SMotaal commented Oct 24, 2018

This thread relates to the first bullet in #196, where during those discussions @jkrems and @SMotaal wanted to find ways outside of the realm of VM for creating a module from SourceText.

My own point of view is that there is a need for a controlled way to construct a module that is linkable via the main loader's ModuleMap as a building block for the loaders. Contrasting this with current experimental-modules and experimental-vm-modules, the first offers createDynamicModule which adds the burden of evaluating the source onto loaders (temp file or forced to non-esm), and the second is not linkable to by modules through the main loader (also not sure if VM adds the usual new context costs/issues to the mix).

@bmeck also mention the discussions taking place in realms, as well as some ideas on how to solve those concerns in order to move forward with loader design in Phase 3.

Related to: Explore design space for virtual module from source

cc @bmeck @jkrems @devsnek

@jkrems
Copy link
Contributor

jkrems commented Oct 24, 2018

Thanks for opening this issue and writing up the summary! :)

@bmeck
Copy link
Member

bmeck commented Oct 24, 2018

@erights is it possible to get invites sent out for Realms discussions related to this?

I agree that we need to allow synthetic bodies from source text to be generated for modules, but I am not convinced that introducing a JS API in the main thread is the way to go about this. Realms had to remove APIs that exposed first class Objects due to oddities while trying to lock things down. Perhaps just the changes described in loader refactoring so that we can return arbitrary formats would supersede the need to link arbitrary modules into the shared module map. I'd rather look down that approach first rather than spend time trying to ensure that user created modules cannot introduce odd side channels.

@SMotaal
Copy link
Author

SMotaal commented Oct 24, 2018

@bmeck, I think we are in the right direction and are nailing down intricacies which are important to move forward where previously we simply might have had some obscurity.

The perspective I am taking is one where I want to see some parity in how a userland loader (emphasis on ESM as much as non-ESM modules) design (irrespective of standards yet to come) can be designed with some level of parity between the browser and node, as a more optimal direction for the community to move forward, and not due to personally liking or disliking established standards that work in the browsers today (ie blobs vs data vs service workers).

I am sure we can continue to hash out the specifics and be able to clearly define the scope of the problem that is best to solve and solve it 👍

Edit: added emphasis bit

@SMotaal
Copy link
Author

SMotaal commented Oct 24, 2018

Sorry @devsnek, I just noticed I did not cc you earlier 😱 (my bad)

@bmeck
Copy link
Member

bmeck commented Oct 25, 2018

We should rename this to something like "Safely creating Modules from raw Source Text". I am still against putting things in the main module map for now unless it is done via a loader. If the intent is to allow the main realm to alter its own loading behavior I would rather put that off to after loader discussions. I think @jkrems might have the inverse opinion but we would still need to solve creating Module from Source Text in both our scenarios.

Per seeing how things can interop today, we should target a few scenariors:

  • HTTP server (HTTP) - rewrite requests on the fly from a server
  • Service Worker (SW) - rewrite requests on the fly from a service worker
  • Ahead Of Time transform (AOT) - ability to bundle/compile things ahead of time

These will be quite difficult if we enable non-string based communication since we have to serialize to a string based request/response system for all of these.

@jkrems
Copy link
Contributor

jkrems commented Oct 25, 2018

If we go for renaming, should we call out the kinds of things we want to enable? If we're creating modules from raw source text but then they just kind of... sit there, I'm not sure what we're gaining (other than a fancier eval in module mode that can't do most of what makes a module a module).

@ljharb
Copy link
Member

ljharb commented Oct 25, 2018

When you say “in the module map”, do you mean static import? I’d assume you could import() a module created from source text, just as if you wrote it to the filesystem and then dynamically imported that.

@bmeck
Copy link
Member

bmeck commented Oct 25, 2018

@jkrems

should we call out the kinds of things we want to enable

seems fine as long as we stick to the subset that everyone wants/feels comfortable moving towards.

@ljharb

both forms of import go through the same mechanism so they shouldn't affect anything.

@SMotaal
Copy link
Author

SMotaal commented Oct 26, 2018

On renaming, I never found out if anyone can do that or just the person who opens the issue, so feel free to try renaming per your own suggestion (first come) then others can discuss since it tracks nicely.

I have no issues with renaming this issue :)

@SMotaal
Copy link
Author

SMotaal commented Oct 26, 2018

I am still against putting things in the main module map for now unless it is done via a loader.

This is the point I keep thinking about, especially when I am doing in browser reflection.

The old days, we just used a loader in a script tag, theoretically this was very different than the idea of a loader in node.

Today, we have an API to define a particular service worker for a particular scope, and if that page is in the scope (maybe I got this wrong) then requests of that page are serviced. I keep confusing this (or just changes over time) but I always use the scope of the page, and to simulate "node_modules" my fetch logic for loader only kicks in once the request includes "node_modules".

This is the extent which I am comfortable to give input on, because my relationship with AOT and back-end (node server) is one of curiosity but client-side (front-end and node) is where I experiment a lot.

@bmeck
Copy link
Member

bmeck commented Oct 26, 2018

maybe we should write up some standard examples of doing things, I tend to write throw away proof of concepts that probably are not up to staying maintained but we could see how they work and then if anything could be changed.

All of these require serialization of resources and cannot use first class values. This might seem limiting at first to not be able to directly talk to the module map.

However, the main use case for virtual resources like vm.Script so far have been testing frameworks and platforms like jsdom. Even if vm.Script doesn't have require() integration it has proven quite useful. Since we need to investigate creating modules from strings in memory for those such use cases, I'd prefer we just use phase 2 for that before we try to tie things into module maps.

@SMotaal
Copy link
Author

SMotaal commented Nov 1, 2018

@bmeck sorry for not getting back sooner... I find your insight on vm.Script a very interesting fact to learn, since others like me don't have these kinds of first hand insights.

I was reflecting on this when I was interrupted by having to take my mac in for repairs and finally set up my very retro one and the analogy was just too great to not use in the response - not that scripts are retro, but they are good for things that happened back then (no intended disrespect to any scripts out there).

From the perspective of a virtual module that is more than vm.Module, I think it is safe to say that none of us are proposing (or would want to see) something like:

import {Module} from '@nodejs/module-which-is-begging-to-be-hacked';

new Module('process', `export const exit = () => import('http').…`);

I also equally feel safe to state on behalf of some (maybe I'm off but not that off) that in some cases new Module(…) with safe-guards is vital to future implementations and use cases of modules, where ModuleMap which did not impact scripts (that was require.cache) is now a fact. But also where insights on scripts, a lot of which was visible and accordingly gained, best serve in the efforts to deliver a module story that is free from pitfalls that such insights inform us enough to avoid.

I think that we have flexibility in dividing our efforts into stages. Phase 2a, 2b... etc. Because Phase 3 is Loaders, so if addressing these concerns happens in 2z then it is material to our effort. But otherwise, it is disruptive to push starting to actually talk loaders in the next phase until we resolve this in a way that lends to a solid loader design, which should be 3a as we hit the ground running.

@SMotaal
Copy link
Author

SMotaal commented Nov 7, 2018

Should we try to schedule some time between today's meeting and the next one to bring to the group something tangible on this?

cc @bmeck @jkrems @devsnek @ljharb

@bmeck
Copy link
Member

bmeck commented Nov 7, 2018 via email

@SMotaal
Copy link
Author

SMotaal commented Nov 7, 2018

Can you elaborate please?

Meanwhile, I am hoping to take a page from how Phase 2-3 evolved from the initial discussion which was a starting point, but was followed up by two 1 hr talks that were conducive to a tangible document to be discussed in the meeting today.

@jkrems is in a better position to share insights on this collaboration and if our efforts could benefit similarly.

@bmeck
Copy link
Member

bmeck commented Nov 7, 2018

@SMotaal I literally just want to go over what me and @jkrems followed up with on a call. I've been writing out a full proposal document based upon that meeting but don't have that in a state that I want to argue about things as it went in a rather different direction than what 2-1 is based upon.

@SMotaal
Copy link
Author

SMotaal commented Nov 7, 2018

Pivoting is an essential part of design, I completely appreciate the process and know how to constructively adapt to big picture aspects — guessing this is one.

Can we reframe this so that we continue on collaborating?

@bmeck
Copy link
Member

bmeck commented Nov 7, 2018

I think that seems like a good discussion topic at a meeting?

@SMotaal
Copy link
Author

SMotaal commented Nov 7, 2018

Whatever it takes, I am serious about reaching the best possible outcomes.

I can set up zoom meetings as needed.

@bmeck

This comment has been minimized.

@SMotaal

This comment has been minimized.

@bmeck

This comment has been minimized.

@SMotaal

This comment has been minimized.

@bmeck
Copy link
Member

bmeck commented Nov 7, 2018

@SMotaal per last meeting, me an Jan were to have an offline meeting about this and if this is something that we need to allow/how. Me and Jan have had a meeting about it, but have ended up with problems discussing this particular issue because of lack of relevant data types and APIs related to it. Hence, I don't really have much to talk about without those. I've been working on writing up those things from our meeting in a proposal that would presumably be outside of the ES Module WG itself since it is needed to exist to talk about these issues. This topic has a lot to discuss but nothing of super relevance to actual implementation planning without various other things like:

  • Determining how to represent location of in-memory resources.
  • Determining how to represent format of in-memory resources.
  • Determining what APIs could be made available that affect reading the resources that are placed into a Module Map.

The big take away is that we don't really have those discussed and they relate to generic resources instead of ESM specific. We can continue to insist on meeting on this topic if desirable, but I'm just going to keep stating that we don't have the related APIs and data types for it to be of meaning. In addition, such APIs need to be generic it seems rather than specific to purely ESM so I'm trying to go around to various parties like React maintainers, the Realms API people, and other existing people related to these generic problems have to say about things.

Per actually putting synthetic resource from in-memory into the module map. I don't really have anything to discuss on this issue due to lack of support structure from other problems that precede talking about this specific issue.

We can and should make a different issue about generic resource APIs but that is out of this WG purview and would likely be done on Node Core itself. That is what I am working on currently, prior to coming to talk about this issue with this group. I've been gathering data prior to submitting any such issue/proposal to Node's Core.

@SMotaal

This comment has been minimized.

@bmeck

This comment has been minimized.

@SMotaal

This comment has been minimized.

@jkrems
Copy link
Contributor

jkrems commented Nov 7, 2018

I think there's been some confusion about what happened. Bradley and myself met, as agreed on, to resolve our fundamental disagreement on what kinds of APIs are safe to expose. We did not make any decisions on this particular item but we did realize some gaps in the basic model of how modules are represented at various stages of the flow.

Bradley then went and wrote up some of these ideas in the form of a more generic system that goes beyond module discussions. This issue might be affected by those discussions but we never got around to actually discuss the details. @SMotaal: I think you're overestimating the degree of discussion that has happened on this front. You didn't miss the discussion - it didn't happen yet. :)

@bmeck
Copy link
Member

bmeck commented Nov 7, 2018

@SMotaal I think we could talk about resource APIs but this issue itself is not ready in any form that I can tell to actually make progress without talking about resources themselves. If we want to talk about this issue, I don't know how to force it to start discussion since we simply don't have the necessary bits to approach this issue.

@SMotaal

This comment has been minimized.

@SMotaal
Copy link
Author

SMotaal commented Nov 8, 2018

@jkrems I appreciate your feedback, and I assure you, it is not a matter of over or understating. There is no problem for discussions to take place, quite the opposite.

@bmeck The only discussion that is problematic is a discussion had elsewhere that renders the discussion being had here immaterial in the conclusion of part of those having it here leaving those not having it there clueless.

The appropriate thing to do is to have an offline discussion to afford your peers the same benefit of reaching the same conclusion or disagree. Because if they do not disagree then the people on this aspect can knowingly inform the group that the discussion needs to be and can be pushed until …

@jkrems Sorry for not being in a position to respond yesterday, but it is not easy to have to ignore remarks made in a live stream so I was working very hard to maintain a positive attitude and not get pulled in such immaterial things. So thank you for bringing in details to help me work.

@bmeck
Copy link
Member

bmeck commented Nov 8, 2018

@SMotaal I am beginning to take offense. I have said at the earliest comment that I would have a few minutes and I can discuss what is going on per your request. Afterwards you insisted for something else. I made note that things from my an @jkrems meetings were not exactly in line with phase 2-1 which is what this issue is about. Afterwards you insisted that we move all discussion here, even though it was noted that I'm not really ready to discuss 2-1. I then made a blurb after feeling a need to explain why / what is not pertaining to 2-1. Afterwards you once again tried to force this issue.

Stop trying to force the issue and stating implied ill intent on my part please. We can still have that meeting, but this form of aggression is toxic. I remain unprepared to discuss 2-1, and have not really made any discussion or progress towards that. Please accept that. I am personally aggravated by your form of aggression and statements of being problematic by my own not being prepared. I would not like to be treated as problematic for attempting to take time to prepare things, think things through, or investigate things on my own. If all discussion must be done in this group, that means that this group effectively is cutting out and time to individually ponder problems.

I am not willing to make this group a forced place for any topics that may even related to Modules. Things like the bullet points above explicitly state what I am trying to research and understand currently. They do not directly pertain to 2-1.

To be clear, there really isn't much to discuss, but your aggression and insistence on discussion being problematic when I clearly stated we could have a meeting and that topics outside of 2-1 are preceding this issue is detrimental to any communication we are trying to achieve on 2-1.

@SMotaal
Copy link
Author

SMotaal commented Nov 8, 2018

I am not trying to force or otherwise, all I wanted and continue to ask for is a little context about the concerns that make this premature to discuss.

I would appreciate it if you can avoid trying to read in my intent, I have avoided commenting about your assumptions, but I see now that it is not helping to not at least clarify that those assumptions are wrong (please let's not get side tracked on this). This is not a venue for this kind of personal dispute, and I would like to respect that and will continue to assume it is what we are all trying to do.

Avoiding miscommunication by a verbal conversation open to all taking part in this discussion, in my humble opinion, is an extension of this thread all the same. Giving us the added benefits of being able to articulate and avoid misreading good intentions under the wrong assumptions.

The fact that you take time to research things is respected by anyone in this group and never questioned nor should you feel that it is.

Please appreciate that I never once commented on "forcing the issue" and I only suggested to give other participants here a to chance to reach a conclusion of their own, no stipulations on what you need to share to get the necessary aspects across.

Recommending verbal conversation was intended to give you the courtesy to not feel pressured by the fact that you are posting this in a thread, which is a discrecion that is also respected by myself and I am sure others.

All I am trying to say, I am with my better intentions making assumptions which also backfire, and the only fair assumption that we both should continue to make is that intentions are in the right place.

So let's move away from things that are unintended and try harder to not to assume things that would stem from nothing but good intents.

When you are ready to give the insights that would make it possible for others to appreciate the problems, maybe even someone can find ways to find solutions to some of those, please let us know how you would like to share this. My hope to report back to the group is only driven by our mutual desire to move on to Phase 3, not force it, that is counter-productive in my own opinion, and that has always been.

Thanks.

@SMotaal
Copy link
Author

SMotaal commented Nov 8, 2018

We did not make any decisions on this particular item but we did realize some gaps in the basic model of how modules are represented at various stages of the flow. — @jkrems

My way of trying to be helpful... I think this is where the context we may be looking may come from when we get to the point of discussion.

@GeoffreyBooth
Copy link
Member

GeoffreyBooth commented Nov 9, 2018

I’m assuming/hoping that @SMotaal and @bmeck worked things out privately . . . would anyone mind if we cleaned up this thread by deleting the somewhat personal comments? I feel mildly uncomfortable scrolling through them as I’m trying to read about the original topic, and I don’t think they need to stay here in public for all to see. This comment can be deleted too as part of any cleanup.

@SMotaal
Copy link
Author

SMotaal commented Nov 11, 2018

@GeoffreyBooth (per messaging you two days ago) this thread has some pending self-moderation efforts. I will start by hiding my own of topic but will leave the more recent and maybe unread ones visible for now.

@SMotaal
Copy link
Author

SMotaal commented Nov 19, 2018

@bmeck I would like to propose starting over in a new issue with a clean slate.

If you prefer, I would recommend that you open issue so that you can reframe the topic and title to articulate our intent and concerns more accurately. While we did not have a chance to discuss things, I am confident that you appreciate the perspectives of everyone in the group.

I hope that @devsnek and @jkrems and others are okay with this forward step.

Sounds good?

@bmeck
Copy link
Member

bmeck commented Nov 26, 2018

I think we can likely move this to a new issue in #231 , @jkrems and myself gave each other a thumbs up on moving forward after reviewing some concerns that were the point of our previous sync with each other.

@SMotaal
Copy link
Author

SMotaal commented Nov 29, 2018

I assumed we can close this issue and move discussions to #231

@SMotaal SMotaal closed this as completed Nov 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants