Add `exports` field to `package.json` #7307

ybiquitous · 2023-11-07T03:48:41Z

Which issue, if any, is this issue related to?

Is there anything in the PR that needs further explanation?

This Pull Request is necessary for Conditional exports (ESM and CJS).

Note: For backward compatibility, we may need to export stylelint/lib/utils/* files.
The following script can easily output many lib/utils/ settings.

Script

import path from 'node:path';

import fg from 'fast-glob';

const exports = {
	'.': {
		import: './lib/index.mjs',
		require: './lib/index.cjs',
	},
	'./package.json': './package.json',
};

const files = fg.sync('lib/utils/isStandardSyntax*.mjs', { ignore: '**/__tests__/**' });

for (const file of files) {
	const base = path.basename(file, '.mjs');
	exports[`./lib/utils/${base}`] = {
		import: `./lib/utils/${base}.mjs`,
		require: `./lib/utils/${base}.cjs`,
	};
}

console.log(JSON.stringify(exports, null, 2));

I'd like to hear feedback about exporting lib/** files. Should we export all files? I worry about frequent breaking changes in the future, if we exposed all modules (including internal ones).

Ideally, we should export utilities only through stylelint.utils, but actually we don't. Especially, isStandardSyntax* utilities are necessary to write a plugin or a custom rule.

This commit is necessary for Conditional exports (ESM and CJS). See https://nodejs.org/api/packages.html#conditional-exports Note: For backward compatibility, we may need to export `stylelint/lib/utils/*` files. The following script can easily output many `lib/utils/` settings. ```js import path from 'node:path'; import fg from 'fast-glob'; const exports = { '.': { import: './lib/index.mjs', require: './lib/index.cjs', }, './package.json': './package.json', }; const files = fg.sync('lib/utils/isStandardSyntax*.mjs', { ignore: '**/__tests__/**' }); for (const file of files) { const base = path.basename(file, '.mjs'); exports[`./lib/utils/${base}`] = { import: `./lib/utils/${base}.mjs`, require: `./lib/utils/${base}.cjs`, }; } console.log(JSON.stringify(exports, null, 2)); ```

changeset-bot · 2023-11-07T03:48:44Z

🦋 Changeset detected

Latest commit: 72d7750

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
stylelint	Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

package.json

Mouvedia · 2023-11-07T14:23:23Z

stylelint/docs/developer-guide/plugins.md

Line 220 in 25140c0

    
           You're also welcome to copy any of the [internal utils](https://github.com/stylelint/stylelint/tree/main/lib/utils) into your plugin. You should not `require` or `import` them directly, as they are not part of the public API and may change or be removed without warning.

Could you also update this paragraph?

ybiquitous · 2023-11-07T15:32:34Z

Could you also update the paragraph here?

Good point. We should clarify the policy for the internal utilities here.

The current documentation says:

stylelint/docs/developer-guide/plugins.md

Lines 216 to 220 in 25140c0

    
           ## `stylelint.utils` 
        
           Stylelint exposes some useful utilities. 
        
           You're also welcome to copy any of the [internal utils](https://github.com/stylelint/stylelint/tree/main/lib/utils) into your plugin. You should not `require` or `import` them directly, as they are not part of the public API and may change or be removed without warning.

stylelint/docs/developer-guide/rules.md

Lines 106 to 113 in 25140c0

    
           #### Utility functions 
        
           Stylelint has [utility functions](https://github.com/stylelint/stylelint/tree/main/lib/utils) that are used in existing rules and might prove useful to you, as well. Please look through those so that you know what's available. (And if you have a new function that you think might prove generally helpful, let's add it to the list!). 
        
           Use the: 
        
           - `validateOptions()` utility to warn users about invalid options 
        
           - `isStandardSyntax*` utilities to ignore non-standard syntax

In my opinion, we should keep the policy that we should expose utilities at a minimum because it may cause frequent breaking changes.

However, if we restricted importing the internals by the exports field, many packages depending on the internals would not work. This should be a pain in the community.

So, I suggest the following:

Keep the copy-paste policy in the documentation. This means an internal utility change won't be a breaking change.
Allow plugin authors to import lib/utils/* modules by exports.

Any thoughts?

Mouvedia · 2023-11-07T15:36:14Z

lib/utils/*
Any thoughts?

I like it because it's not restrictive and * will do the job for us; i.e. no maintenance needed.

ybiquitous · 2023-11-07T16:17:23Z

Fixed to lib/utils/* with e1597ea

ybiquitous · 2023-11-07T16:24:10Z

The following is a test of ./lib/utils/* in the exports field.

package.json:

{
  "dependencies": {
    "stylelint": "github:stylelint/stylelint#add-exports-field"
  }
}

Run:

$ npm install
...

$ node -pe 'require("stylelint/lib/utils/typeGuards.cjs")'
{
  hasSource: [Function: hasSource],
  isAtRule: [Function: isAtRule],
  isComment: [Function: isComment],
  isDeclaration: [Function: isDeclaration],
  isDocument: [Function: isDocument],
  isRoot: [Function: isRoot],
  isRule: [Function: isRule],
  isValueFunction: [Function: isValueFunction]
}

$ node -e 'import("stylelint/lib/utils/typeGuards.mjs").then(console.log)'
[Module: null prototype] {
  hasSource: [Function: hasSource],
  isAtRule: [Function: isAtRule],
  isComment: [Function: isComment],
  isDeclaration: [Function: isDeclaration],
  isDocument: [Function: isDocument],
  isRoot: [Function: isRoot],
  isRule: [Function: isRule],
  isValueFunction: [Function: isValueFunction]
}

package.json

Mouvedia

LGTM

package.json

jeddy3

I'd like to hear feedback about exporting lib/** files. Should we export all files? I worry about frequent breaking changes in the future, if we exposed all modules (including internal ones).

In #6866 (comment), we settled on not exporting more utils (e.g. because people shifting to standard CSS makes the isStandardSyntax* utils less relevant and so on).

Am I right in thinking that our move to ESM will break people's imports anyway, regardless of whether we export the utils as the extensions have changed:

- require("stylelint/lib/utils/typeGuards.js")'
+ require("stylelint/lib/utils/typeGuards.cjs")'

Rather than apply another bandaid, we could take this opportunity to make it clear in our migration guide that plugin authors need to copy any internal utils they want (like stylelint-scss does). This clarification is likely worth it as the ambiguity has been problematic in the past, e.g. with stylelint-stylistic.

This will also address #7307 (comment), where changing back to .js would be another breaking change for those importing internal .mjs utils.

ybiquitous · 2023-11-08T11:17:03Z

@jeddy3 Thanks for the feedback. I also don't want to change the policy we settled on #6866 (comment).

And yes, the following code would break in 16.0.0:

require("stylelint/lib/utils/typeGuards.js")

But, if we have ./lib/utils/* like #7307 (comment), the following code will NOT break (the difference is just the existence of .js):

require("stylelint/lib/utils/typeGuards")

On the other hand, if we do NOT provide ./lib/utils/*, the code above will break, that is the following error will occur:

Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './lib/utils/typeGuards' is not defined by "exports"

I'm worried that this breaking might be too strict. So I suggest:

In 16.0.0, we deprecate the use of internal importing like require('stylelint/lib/**') or import 'stylelint/lib/**', but allow the use though exports.
In 17.0.0, we remove exports for ./lib/**.

Any thoughts?

jeddy3 · 2023-11-08T12:25:35Z

I'd assumed most people were using the .js extension, but (after a cursory look) it seems most aren't.

I'm worried that this breaking might be too strict

Your suggestion SGTM. It'll stagger the amount of people affected over two major releases, with the most happening in 17.0.0.

And yes, the following code would break in 16.0.0:

Let's add something to the migration guide as part of this pull request for this. And let's also use the opportunity to steer people in the right direction. Something along the lines of:

Changed extension of internal files

We've changed the extension of our internal files from .js to .mjs and .cjs. If you're a plugin or integration author that imports or requires any internal files (e.g. our util/* ones), we recommend copying these to your project instead as we:

intend to disallow access to them in the next major release
may remove and change files that are not part of our public API outside of a major release

We don't recommend it, but you can unsafely continue to import or require the files until the next major release by using the appropriate new extension. For example:

- require("stylelint/lib/utils/typeGuards.js")'
+ require("stylelint/lib/utils/typeGuards.cjs")'

- import isStandardSyntaxAtRule from 'stylelint/lib/utils/isStandardSyntaxAtRule.js';
+ import isStandardSyntaxAtRule from 'stylelint/lib/utils/isStandardSyntaxAtRule.mjs';

ybiquitous · 2023-11-08T14:01:34Z

@jeddy3 Sounds good! I've updated the migration guide (565ba5d) and added a changelog entry (72d7750). Please take a look at them. 🙏🏼

ybiquitous · 2023-11-08T14:02:53Z

Feel free to modify the documents directly if necessary. I'm not so confident in my English wording. 😅

Mouvedia

Talking about the non-addition of type, here's an example of where it would have been useful: if we were keeping the utils extension as .js to avoid breaking changes. In this scenario it would have been "type": "commonjs" though.
Like @jeddy3, I was assuming—wrongly?—that extension-less usage would be rare. If it's really not the case then reverting to .js from .cjs for this folder might not be necessary.

ybiquitous · 2023-11-08T14:57:46Z

For your information, suppose we provide pure ESM in 17.0.0 (the next to 16.0.0).

package.json will be:

{
  "type": "module",
  "exports": {
    ".": "./lib/index.js",
    "./package.json": "./package.json"
  }
}

.mjs will be converted to .js
.cjs will be removed

Then, most codes depending on stylelint should be broken:

import("stylelint"); //=> ✅ Not broken
import("stylelint/lib/utils/typeGuards.mjs"); //=> ❌ Broken due to invalid `.mjs`
import("stylelint/lib/utils/typeGuards"); //=> ❌ Broken due to no `exports`
require("stylelint"); //=> ❌ Broken due to ERR_REQUIRE_ESM

Mouvedia · 2023-11-08T15:08:09Z

.mjs will be converted to .js

I don't think that's necessary. Once we have "type": "module", both .js and .mjs can coexist.
In v17, at least the main index.mjs and the utils should remain .mjs to smooth the transition for the users.
Anyhow, that will probably be discussed in the upcoming Prepare 17.0.0.

ybiquitous · 2023-11-08T15:15:56Z

.mjs will be converted to .js

I don't think that's necessary. Once we have "type": "module", both .js and .mjs can coexist.

Ah, my bad. You're correct.

jeddy3

LGTM, thank you!

ybiquitous linked an issue Nov 7, 2023 that may be closed by this pull request

Use exports field in package.json to forbid unexpected API usage #6258

Closed

Add changelog

97a3b95

ybiquitous commented Nov 7, 2023

View reviewed changes

package.json Show resolved Hide resolved

ybiquitous marked this pull request as ready for review November 7, 2023 03:54

ybiquitous mentioned this pull request Nov 7, 2023

Fix incorrect main field in package.json #7308

Merged

Mouvedia reviewed Nov 7, 2023

View reviewed changes

package.json Show resolved Hide resolved

Mouvedia reviewed Nov 7, 2023

View reviewed changes

package.json Outdated Show resolved Hide resolved

Use subpath patterns for lib/utils/isStandardSyntax* utilities

027567a

ybiquitous added 2 commits November 8, 2023 01:11

Merge branch 'v16' into add-exports-field

49b21aa

Expose all lib/utils/* files via exports

e1597ea

This was referenced Nov 7, 2023

Change TypeScript definitions for ESM #7309

Merged

Prepare 16.0.0 #6930

Closed

Mouvedia reviewed Nov 8, 2023

View reviewed changes

package.json Outdated Show resolved Hide resolved

Remove type field

9a6621c

ybiquitous changed the title ~~Add type/exports fields to package.json~~ Add exports field to package.json Nov 8, 2023

Mouvedia approved these changes Nov 8, 2023

View reviewed changes

ybiquitous commented Nov 8, 2023

View reviewed changes

package.json Show resolved Hide resolved

jeddy3 reviewed Nov 8, 2023

View reviewed changes

ybiquitous added 2 commits November 8, 2023 22:58

Update migration guide

565ba5d

Add changelog

72d7750

ybiquitous requested a review from jeddy3 November 8, 2023 14:07

Mouvedia approved these changes Nov 8, 2023

View reviewed changes

jeddy3 approved these changes Nov 8, 2023

View reviewed changes

ybiquitous merged commit 310c9ad into v16 Nov 9, 2023
14 checks passed

ybiquitous deleted the add-exports-field branch November 9, 2023 00:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `exports` field to `package.json` #7307

Add `exports` field to `package.json` #7307

ybiquitous commented Nov 7, 2023 •

edited

changeset-bot bot commented Nov 7, 2023 •

edited

Mouvedia commented Nov 7, 2023 •

edited

ybiquitous commented Nov 7, 2023 •

edited

Mouvedia commented Nov 7, 2023 •

edited

ybiquitous commented Nov 7, 2023

ybiquitous commented Nov 7, 2023

Mouvedia left a comment

jeddy3 left a comment •

edited

ybiquitous commented Nov 8, 2023

jeddy3 commented Nov 8, 2023 •

edited

ybiquitous commented Nov 8, 2023

ybiquitous commented Nov 8, 2023

Mouvedia left a comment •

edited

ybiquitous commented Nov 8, 2023

Mouvedia commented Nov 8, 2023 •

edited

ybiquitous commented Nov 8, 2023

jeddy3 left a comment

Add exports field to package.json #7307

Add exports field to package.json #7307

Conversation

ybiquitous commented Nov 7, 2023 • edited

changeset-bot bot commented Nov 7, 2023 • edited

🦋 Changeset detected

Mouvedia commented Nov 7, 2023 • edited

ybiquitous commented Nov 7, 2023 • edited

Mouvedia commented Nov 7, 2023 • edited

ybiquitous commented Nov 7, 2023

ybiquitous commented Nov 7, 2023

Mouvedia left a comment

Choose a reason for hiding this comment

jeddy3 left a comment • edited

Choose a reason for hiding this comment

ybiquitous commented Nov 8, 2023

jeddy3 commented Nov 8, 2023 • edited

Changed extension of internal files

ybiquitous commented Nov 8, 2023

ybiquitous commented Nov 8, 2023

Mouvedia left a comment • edited

Choose a reason for hiding this comment

ybiquitous commented Nov 8, 2023

Mouvedia commented Nov 8, 2023 • edited

ybiquitous commented Nov 8, 2023

jeddy3 left a comment

Choose a reason for hiding this comment

Add `exports` field to `package.json` #7307

Add `exports` field to `package.json` #7307

ybiquitous commented Nov 7, 2023 •

edited

changeset-bot bot commented Nov 7, 2023 •

edited

Mouvedia commented Nov 7, 2023 •

edited

ybiquitous commented Nov 7, 2023 •

edited

Mouvedia commented Nov 7, 2023 •

edited

jeddy3 left a comment •

edited

jeddy3 commented Nov 8, 2023 •

edited

Mouvedia left a comment •

edited

Mouvedia commented Nov 8, 2023 •

edited