Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: add source community condition definition #43376

Closed
wants to merge 1 commit into from

Conversation

guybedford
Copy link
Contributor

@guybedford guybedford commented Jun 11, 2022

I'd like to propose a new "source" condition in the "exports" field.

Currently, Node.js applications tend to assume that what is published to npm and imported from node_modules should be readable JS code and generally isn't minified and optimized by default.

In Node.js this is because minification and optimization doesn't provide a huge benefit for runtime performance.

On the other hand, when running JS code in the browser, minification and single-file optimizations are an absolute necessity.

When specifying a "browser" exports condition in the package.json, there's then a sort of implicit assumption that one should reference a minified source file, in contrast to an unminified version for Node.js.

This disparity is something package authors often have to wrestle with in making individual decisions for environments in this way.

A "source" condition would be defined as the uncompiled version of the code that runs the original source directly, useful for debugging workflows. By permitting packages to define this condition, we are encouraging packages to publish their original sources beyond just questionable source maps support, but actually in a form that can allow debugging back to the original readable code in all environments, while still being able to publish optimized JS code to npm.

The existing "development" and "production" conditions don't capture this condition because they refer to the execution mode, not the execution performance. Things like single-file-optimizations, minification and removing comments can be seen as optimizations that apply to both builds.

By using a "source" condition over a "optimized" condition or otherwise, we make it clear that the default mode for sources should really be to publish the most optimized version, and that switching back into a source condition is something that is done under special circumstances.

I've found this pattern incredibly useful for local development, running local tests via both node test/test.js and node -C source test/test.js when source maps are not sufficient to trace the error, and live code editing to add comments or further analysis is needed directly on the original sources to track the bug.

//cc @nodejs/modules

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/modules

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Jun 11, 2022
@GeoffreyBooth
Copy link
Member

But what if the original source isn't runnable, like TypeScript? Is the expectation that source is unminified/unobfuscated but still runnable, whereas any "original" source that isn't runnable without transpilation is under some other field?

@ljharb
Copy link
Member

ljharb commented Jun 11, 2022

This is something npm should handle, not node, with the package distributions RFC.

@bnb
Copy link
Contributor

bnb commented Jun 11, 2022

By permitting packages to define this condition, we are encouraging packages to publish their original sources beyond just questionable source maps support, but actually in a form that can allow debugging back to the original readable code in all environments, while still being able to publish optimized JS code to npm.

I'm not going to lie, I don't know if this is something I'd want to encourage. Specifically, I think this exact thing will dramatically increase total bundle sizes resulting in both increased size on disk for end-users (I care more about the total disk usage on personal/work computers, not servers where stuff is running) and it will globally increase install times.

IMO the issues I mentioned are 100% because the npm registry does not provide a way to offer multiple distributions of the same version of a package (I've had this discussion many times with @ljharb, begging him to drop tests from his modules). I would genuinely love it if they did (then I wouldn't have to have @ljharb's and others' tests in my node_modules, but they could have them if they wanted to). I'm concerned about Node.js (or any other runtime/platform!) trying to solve this with features because it's fundamentally a workaround for a registry issue, and I'd hope that instead of shipping something that works around the problem we can convince the problem to be solved at the source of it.

IMO this isn't a bad feature and I wouldn't be upset if it shipped, but I think there are more direct solutions that are better for the ecosystem and don't introduce features that we'll have deprecate/change/teach not to use as a part of best practices later.

@guybedford
Copy link
Contributor Author

But what if the original source isn't runnable, like TypeScript? Is the expectation that source is unminified/unobfuscated but still runnable, whereas any "original" source that isn't runnable without transpilation is under some other field?

I actually use this pattern on a TypeScript codebase myself, and the benefit during development is only having to run tsc but no other compilation build steps so that the dev-refresh workflow becomes pretty instant - you're just compiling the changed files with the TypeScript watcher, and since there's no minification or bundling of files the JS remains very human readable.

This is something npm should handle, not node, with the package distributions RFC.

Can you explain how npm should handle this in more detail, or link to the source? I'm not aware of these efforts and it is definitely worth comparing the pros and cons here more carefully.

I'm not going to lie, I don't know if this is something I'd want to encourage. Specifically, I think this exact thing will dramatically increase total bundle sizes resulting in both increased size on disk for end-users

Here's one way to frame it - should it be possible to edit files in node_modules to debug an application? And should that ability stop npm package authors from publishing well-optimized packages? Let's perhaps focus on the problem before shooting down solutions, without understanding what these costs are being weighed against.

IMO this isn't a bad feature and I wouldn't be upset if it shipped, but I think there are more direct solutions that are better for the ecosystem and don't introduce features that we'll have deprecate/change/teach not to use as a part of best practices later.

I would be grateful for links / resources further.

@GeoffreyBooth
Copy link
Member

I actually use this pattern on a TypeScript codebase myself

So should sources be the TypeScript files or a runnable readable output? It's not clear to me either from the comments on this thread or in the PR, and I think it needs to be crystal clear in the docs if we add this.

@guybedford
Copy link
Contributor Author

So should sources be the TypeScript files or a runnable readable output? It's not clear to me either from the comments on this thread or in the PR, and I think it needs to be crystal clear in the docs if we add this.

I personally recommend the following workflow for performance and ease of development:

- src/**.ts files
- lib/**.js files
- dist/**.js files optimized

Where the src files are original TypeScript, the lib files are exact 1-1 compilation of those TypeScript files, and the dist files are the optimization.

I then use the following package.json file:

{
  "exports": {
    "source": "./lib/index.js",
    "default": "./dist/index.js"
  }
}

During development, I run TSC compilation through the simple TSC watcher and run tests and example applications with the node -C source invocation so that I don't need to create optimized builds in development (which take seconds to make, seconds I don't want to waste during a dev cycle).

When publishing to npm, I only then run the optimization, and use the fact that node -C source test.js just becomes node test.js to run tests equivalently on the optimized version - this is a big benefit of utilizing conditional resolution here in that it integrates at the same level.

When linking the package into another dependency using npm link or otherwise, it can be useful to attach the same dev workflow to original sources in the same way which then all works out.

I think we need to treat original sources as a very important supply chain security property of the npm ecosystem, and actively protect that. I hope we at least agree on that!

@bnb
Copy link
Contributor

bnb commented Jun 11, 2022

I would be grateful for links / resources further.

I believe @ljharb was referring to this RFC which does not do exactly what he/I would want, but conceptually gets close: npm/rfcs#519

Can you explain how npm should handle this in more detail, or link to the source?

Conceptually, I believe that the npm Registry should allow you to publish multiple distributions (the specific name doesn't matter, distributions is just a somewhat accurate representation) for the same version. For example:

  • I have module x
  • I ship [email protected]
  • I decide I want [email protected]'s default distribution to be the smallest possible package so it requires the least disk space and data over the wire as is possible.
  • However, I want my users to be able to have [email protected] with tests so they can run them. I would publish a [email protected] with-tests distribution that would have the minified version with tests.
  • I could also publish a [email protected] with-tests-and-source distribution that includes both my tests and the source code that default was built from.

This has a some nice benefits:

  • There can be multiple variants provided by maintainers who want to give that to their users
  • It allows smaller packages without losing the fidelity of including tests/source/whatever else you want to include
  • It's additive to the ecosystem - it doesn't need to change what we already do/what we already publish

My personal perspective is that enabling this within the existing publishing structure (as this PR aims to do) does solve the problem but in a way that is less optimal than the underlying platform itself supporting the use case.

Definitely want to note that I'm not blocking this or trying to toss out the proposed solution, just want to surface my recommendation that this be done at the registry level rather than the Node.js level for what I think would be a more robust solution to the (extremely valid!) problem.

@guybedford
Copy link
Contributor Author

It's difficult to come up with a single solution to this problem of maintaining the original JS source supply chain on npm - I think rather than trying to point to some future proposal which will fix everything it can be better to think in terms of primitives that can enable use cases.

As an interpreted language that is also optimized, this is a uniquely JS problem.

I see different approaches at the package management and source levels being able to work together and compose and depend on eachother's behaviours.

I do think that "source" is one useful primitive here, and while I agree that it isn't something we should make as an explicit recommendation for everyone to do, having this condition at least defined with a strict semantic meaning (per the conditions definitions requirements) will allow ecosystem support for such a primitive to further explore its application.

As the future points towards optimized packages being published to npm as a potential best-practise, I think we need to work to create these primitives, and not hinder work that will help keep JS having the properties that we all know and love.

Another approach might just be for a GitHub clone of the original repo to have a direct workflow something like the convention:

  • For any published package "x", cloning the GitHub "repository" and then cd'ing into the "package" folder (say per some "directories" definition) and running npm run build will provide a folder that behaves identically to the original but that can be edited by anyone.

One issue with these though is since they are additive to existing workflows, it is very hard to get alignment on such things and in practice most packages end up needing custom steps.

This is where "source" condition offers a benefit for the small subset of tools, toolchains or custom workflows for Node.js that I'd want to provide. In particular I'd like to work on a package optimizer that takes as input a package with exports, and provides as output an optimized version of that package folder. If we had a "source" condition, this optimization process would be able to provide an optimization toolchain that doesn't destroy the important "back-to-source" referencing we have in Node.js today, which isn't a given for the future.

Comment on lines +663 to +665
* `"source"` - can be used to reference unminified / unoptimized source versions
to use for debugging workflows, that will behave identically to any non-source
variants.
Copy link
Member

@GeoffreyBooth GeoffreyBooth Jun 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* `"source"` - can be used to reference unminified / unoptimized source versions
to use for debugging workflows, that will behave identically to any non-source
variants.
* `"source"` - can be used to reference unminified, unoptimized, _runnable_
source JavaScript files. These can be used for debugging workflows, that will
behave identically to any other variants.

I’m not sure if source is the best name for this, since many people might assume that source should refer to original TypeScript or other before-building files. Perhaps pretty, to correspond with DevTools pretty printing minified source files?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only other word I can think of off-hand that captures the same meaning might be "unoptimized".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"pretty" is close, but it's not just unminification, it's also having separated source files structured by the author.

@ljharb
Copy link
Member

ljharb commented Jun 12, 2022

I also think that a single “source” would replicate the disaster that is the RN ecosystem to the rest of JS - where folks rely on an arbitrary build process/config to compile their node_modules and things randomly break when authored with another one in mind.

This isn’t node’s problem to solve.

@guybedford
Copy link
Contributor Author

where folks rely on an arbitrary build process/config to compile their node_modules and things randomly break when authored with another one in mind.

This proposal does not imply any build process for node_modules. The reason I want to define a "source" condition's meaning is exactly because unless we have a clear unambiguous meaning for the condition, it risks randomly breaking if someone else defines its meaning. I want to publish packages using this condition, and avoid breaks by ensuring the condition's runtime meaning (where the condition always works in Node.js either on or off) is strictly defined.

This isn’t node’s problem to solve.

As a community condition definition this is not a Node.js feature, it is a conditional exports shared ecosystem convention space that Node.js is still stewarding in its documentation for now.

@ljharb
Copy link
Member

ljharb commented Jun 12, 2022

Right, but the community hasn’t defined this one. Node should be documenting community conventions, not dictating them.

@guybedford
Copy link
Contributor Author

Sure, I can go ahead with the condition in my own tooling and come back to the definition once there is more usage.

@guybedford guybedford closed this Jun 12, 2022
@guybedford guybedford deleted the source-condition branch June 12, 2022 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants