This repository has been archived by the owner on Sep 2, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 42
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #439 from nodejs/minutes-2019-11-20
Minutes for 2019-11-20 meeting
- Loading branch information
Showing
1 changed file
with
104 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,104 @@ | ||
| # Node.js Foundation Modules Team Meeting 2019-11-20 | ||
|
|
||
| ## Links | ||
|
|
||
| * **Recording**: http://www.youtube.com/watch?v=6l55SFZWiQw | ||
| * **GitHub Issue**: https://github.com/nodejs/modules/issues/437 | ||
| * **Minutes Google Doc**: https://docs.google.com/document/d/1zK7R1qwmAtdf1GzF3EvoUDCX_nI7Eoc_My6HweBdGTs/edit | ||
|
|
||
| ## Present | ||
|
|
||
| * Myles Borins (@MylesBorins) | ||
| * Rob Palmer (@robpalme) | ||
| * Bradley Farias (@bmeck) | ||
| * Saleh Abdel Motaal (@SMotaal) | ||
| * Guy Bedford (@guybedford) | ||
| * Jordan Harband (@ljharb) | ||
| * Jan Krems (@jkrems) | ||
| * Corey Farrell (@coreyfarrell) | ||
| * Evan Plaice (@evanplaice) | ||
|
|
||
| ## Agenda | ||
|
|
||
| ## Announcements | ||
|
|
||
| * Extracted from **modules-agenda** labelled issues and pull requests from the **nodejs org** prior to the meeting. | ||
|
|
||
| ### Group Members | ||
|
|
||
| - Adding Corey Farrell as Observer | ||
| - **CONCLUSION:** Approved | ||
|
|
||
| ### nodejs/node | ||
|
|
||
| #### esm: The `getPackageType` utility function is not exposed [#30514](https://github.com/nodejs/node/issues/30514) | ||
|
|
||
| - BF: Build tools want to know the format of files inside a package boundary. We could export the `"type"` field via a user-land API queryable by Node. Opinions differ on the request and the API design considerations. CoffeeScript wants `.coffee` files to match whatever the format of a `.js` file is. Want to determine what a `.js` file would be, if it were to exist, so that coffee files can compile to that format. History: coffeescript does not control its build artifacts - asset pipelines do this. Pipeline generates `.js` files - they just want to stay true to that. Comments from TypeScript suggest they like an API design. Consideration: expose the type field or the database mappings in the runtime. I think either works, but have concerns about misuse and forwards compaibility if we do not expose enough. Will lead to disparate systems Example: an issue neglected that `.cjs` doens't populate in the internal ext->format database mapping. Repeated in the thread, when TypeScript want to add file extensions specific to TypeScript, to retain today's experience. This means tools need to cooperate with TypeScript somehow. so this is about coordinating tools. Real world example: ESLint did not have this API, so it attempts to load files in odd ways to get the RC files to load with `--experimental-modules`. We have an old issue in this WG, about exposing configuring the database from package.json files - the issue mentions this. | ||
| - BF: Do people think it's ok to expose this machinery? | ||
| - MB: An API that would be useful would be one that takes a path and tells you which format Node will interpret that format as. It doesn't necessarily parse the file contents. Is this inline with the proposed API? | ||
| - BF: It is discussed in the issue in two forms. Expose the database at a location. Other is going from location to a format. | ||
| - JH: Does it have to exist? | ||
| - BF: Presumably no. In coffeescript case, the file will not exist. | ||
| - JH: Could be tricky if you query for a path that does not exist. Maybe it could return an array of answers. | ||
| - MB: Extensionless fallsback to CJS today. So that's the answer. | ||
| - CF: CJS will resolve index.js | ||
| - JH: API would need to be able to return multiples. | ||
| - BF: Does it do searching? | ||
| - JH: Which loader does it use? (no need for real file or extension searching - just picks a loader). Or you say "what module format is this?" it's trickier. One world: speculative. Other world: Based on filesystem, but that does not help coffeescript. | ||
| - BF: Thread has not discussed resolving to a different location. Only looks at the import loader. | ||
| - JK: More important thing is that Guy Bedford is correct - it must be a userland thing. TS and webpack want this. They do not want to hit the filesystem because they have their own abstraction. Gut feeling is we could expose it and get locked into the implementation without helping any of the users, because they need to work with a virtual filesystem. Officially supporting an npm package that does this... maybe? But not useful as a core package. TypeScript would only work on a version of Node that provided it. | ||
| - JH: If I build this userland library today, do I need to replicate resolution logic? Could Node expose building blocks for this. | ||
| - JK: I assume this is not about resolution. Your path is already resolved. Queries are already fully resolved, even if it did not exist. The thread, Guy's example, it's just encoding the logic of how we interpret the metadata. It's a very small amount of code, returning a small bit of information. The complexity is low - no need for a core API. | ||
| - SM: I don't like the speculation when dealing with the filesystem. It will require a lot of coordination and will be biased. Node does two things. Discovers package.json. Then it know resolution approach for extensions. So two APIs. 1. Get the mode. 2. Then resolve using mode. These two parts are stable. I believe this covers most tools' use-cases. | ||
| - CF: I'm not sure this is the API that would be useful to me. I want to know the ultimate result of the resolve. Which includes the result of loader hooks. It's more important than what package.json says. | ||
| - MB: We have the mechanisms and infrastructure in Node, to look at a file, and decide which transform it goes through. It is consistent and internal. Exposing it seems reasonable. Many people will try to emulate this and may get it wrong. The API "what would we do if we tried to load this path?" sounds reasonable. | ||
| - BF: We've talked a lot about "should we expose it?". What about "should we customise the database?" | ||
| - GB: I want to stress that the resolve implementation is expected to lead to many userland implementations of the resolve. Hence we have a clear spec. The spec is the source of truth, not the code. So implementation drift is mitigated by this spec. Maybe we need to make the spec more prominent. | ||
| - MB: Implementation and Spec are good - but it need tests, just like TC39 does it. We need a test suite. | ||
| - GB: Agreed. | ||
| - BF: Just because we have a spec, people may not follow it to the letter. Userland extensions happen. New formats TS, JSX, are going to break our spec, because they have no way to coordinate. Implementations will go out of date. You could be running an old version of TypeScript that does not handle the full set today's Node has. HTTP exists as a spec, but it does not mean people want to run multiple uncoordinated implementations. If we do not provide loaders with a way to get the format, we have not met our goal. A spec alone is insufficient. We need a way of coordinating. | ||
| - JK: I agree with Bradley. Loaders want to know about currently active extensions. But pre-compilation needs it too. Having implementation in Node, means you need to run it with the same flags as the app. So TypeScript needs it via a different channel. It would not use the API even if it existed. | ||
| - SM: We want the logic available ambiently. | ||
| - BF: Counterpoint to Jan. If TS does precomp to a supported format, those things are not needed at runtime. | ||
| - JK: But is needed at compile-time. | ||
| - BF: That is true for loaders. | ||
| - JK: *will take this offline* | ||
| - What considerations do we have about exposing the raw field? | ||
| - How do people feel about exposing the ability for users to configure these databases of extension->format (not necessarily via package.json). Then we can figure out how to configure it. | ||
| - **CONCLUSION:** Keep talking on the Github issue | ||
|
|
||
|
|
||
| ### nodejs/modules | ||
|
|
||
| #### Module Attributes RFC [#427](https://github.com/nodejs/modules/issues/427) | ||
|
|
||
| - MB: Was raised at TC39 in response to WHATWG/W3C feedback at TPAC where JSON modules were rejected due to security concerns. Importing a JSON file Node will parse during fetch and will return a default value. Security concern was that you relied on the server declaring the correct MIME type. Server could maliciously return JS instead of JSON - turns data into executable code. Issue was kicked back to committees. This is an in-band solution. Using extra syntax to assert the type of the module being loaded. | ||
| - JH: It's premature to discuss here prior to TC39. I do not think syntax change will get consensus there. I do not buy the threat model - if you don't trust it, don't import it. Same thing as pulling JS from a different domain - good JS can be swapped to bad JS. I think Node and Browser concerns are different here. Node should not degrade UX. | ||
| - BF: Minor correction - this is not specifying the type of the import, it's restricting the allowable types. I am uncomfortable saying we should match the web's security model. I am unclear how this affects Node and why we should do it. The unspoken concern: if the web requires this, Node must support it (or ignore them) to retain web compatibility. I think both outcomes are problematic. I have stated concerns on the repo. I do not think the threats are realistic. Only minor games - one level deep import from an untrusted origin. Other things can expand the dependency graph. Wasm and CSS have issues. These seem to be an odd solution to a problem that Node does not have. Maybe even web is not solving the problem they want to solve. We must decide: can we implement it? and should we implement this? I think we can but should not. The threat model does not apply to Node. | ||
| - MB: Jordan, you said it's premature. I disagree - we need feedback from this group to take to TC39. Brad says, this could cause Node/Browser incompatibility. Don' agree with Brad that this has nothing to do with Node - we may start loading from HTTP. Then the security becomes similar. If we want JSON modules on the web, we need a palpable solution for the browsers. We must come up with something. Otherwise we agree to drift, or just live with no JSON modules on the web. | ||
| - JK: I am concerned about the current direction of the proposal. Type assertions are bad, even on the web. The use-cases outlined are questionable. Protecting everything *except* JavaScript seems weird. A basic assertion to say "this is just static data" seems better. Listing allowable file types does not seem like it will get much take-up. | ||
| - BF: Be careful about talking about HTTPS on Node. Deno did this but they have a different sandbox model. Node could not use the Web's model. Security Working Group has discussed this. | ||
| - JK: It's not about origins. Just about saying file in non-executable. | ||
| - BF: Trusted origins is a big part of this proposal. | ||
| - JK: This proposal says you need to annotate the untrusted ones. It doesn't even rely on the web security model. | ||
| - BF: Do we have implementation concerns? | ||
| - SM: Where would we implement this? | ||
| - BF: A format check at the end of the load operation. Parse time ingestion, passed through host hooks. | ||
| - JK: A place where this is problematic is the CJS bridge. We cannot forward this info easily. We'd treat all CJS as modules. That's the only implementation concern. | ||
| - MB: Jordan, you do not believe the proposal has legs? | ||
| - JH: I do not expect to be a lone objector on this. The entire design of the import syntax and the security model of the web is just not going to fit together here. We can talk about it more independently. I would not block it as a lone objector. | ||
| - MB: As a group, we should think about out-of-band alternative solutions. | ||
| - **CONCLUSION:** It's technically implementable. Open question as to whether we should. | ||
|
|
||
| **END OF MEETING** | ||
|
|
||
| --- | ||
|
|
||
| #### Chartering the Modules team [#412](https://github.com/nodejs/modules/issues/412) | ||
|
|
||
| - Not discussed | ||
|
|
||
| #### Loader Hooks [#351](https://github.com/nodejs/modules/issues/351) | ||
|
|
||
| - Not discussed | ||
|
|