From 3f6e7bcdd79726364fcbef41e5bca233a33e8521 Mon Sep 17 00:00:00 2001 From: cvanem Date: Fri, 17 May 2019 02:40:01 -0600 Subject: [PATCH 1/6] Add importable scripts --- accepted/0000-importable-scripts.md | 153 ++++++++++++++++++++++++++++ 1 file changed, 153 insertions(+) create mode 100644 accepted/0000-importable-scripts.md diff --git a/accepted/0000-importable-scripts.md b/accepted/0000-importable-scripts.md new file mode 100644 index 0000000..8d5a303 --- /dev/null +++ b/accepted/0000-importable-scripts.md @@ -0,0 +1,153 @@ +- Start Date: 2019-05-17 +- RFC PR: +- Yarn Issue: + +# Summary + +Allow scripts to be imported from a file as an alternative to the standard +object hash in the packages.json scripts field. Also define the basic +structure of the imported file. + +# Motivation + +The main motivation behind this is to allow scripts to be better organized +and documented in one place. This can be useful for large projects with +numerous scripts. It is also helpful for simplifying auto-generated +documentation and organizing scripts with comments or associated metadata. +This is not possible with the current package.json structure. If implemented, +this feature allows the following: + + 1. Ability to write comments next to an associated script. + 2. Ability to include metadata with an associated script. + 3. Ability to structure and group scripts. + +# Detailed design + +The intended design allows a filename to be specified in the packages.json +scripts field as an alternative to the default scripts object. This would then +be used to import scripts making them available to yarn via the current manifest +structure. + +The actual implementation would occur directly after the manifest is read, in +the tryManifest function. The new design would check the scripts field to +determine if it is an importable file. If it is, then the scripts would be +imported and effectively replace the filename in the manifest with the imported +scripts list. + +For the basic structure used for the imported script module, I propose that it +is defined as follows: + + module.exports = { + scripts: { + ... + }, + }; + +For defining scripts, I propose supporting the following structures: + +1. Structure as a standard object hash inside of the scripts object. This + would be identical to how scripts are currently specified in the + packages.json file with the additional ability of adding comments. + + module.exports = { + scripts: { + "test": "cross-env CI=1 react-scripts test --env=jsdom", // Run tests + "test:watch": "react-scripts test --env=jsdom", + "build": "rollup -c", // Builds the library + "start": "rollup -c -w", // Build and watch the library + }, + }; + +2. Structure a script as an object with a 'script' key. This allows for additional + metadata, other than comments, to be co-located with an associated script. When a + script key is specified within an object, the object's remaining non object values + are ignored. Remaining objects are imported imported as normal. + + module.exports = { + scripts: { + "test": { + script: "cross-env CI=1 react-scripts test --env=jsdom", + description: "Run tests using react-scripts", + }, + "test:watch": "react-scripts test --env=jsdom", + "build": "rollup -c", + "start": "rollup -c -w", + }, + }; + +3. Group scripts together using objects and allow a 'default' key. When scripts are grouped + together, the script name or key will be generated from the object hierarchy. For example, + the test watch script below would be exposed via the 'yarn test.watch' command. + + If a script is specified under an object's 'default' key, that script name will inherit + it's parent's name. Therefore, in the below example, the test.default script would be + exposed via the 'yarn test' command. + + module.exports = { + scripts: { + "test": { + default: "cross-env CI=1 react-scripts test --env=jsdom", + watch: "react-scripts test --env=jsdom", + }, + "build": "rollup -c", + "start": "rollup -c -w", + }, + }; + +Checks and warnings will be put in place during script imports to flag any duplicate keys. +This can happen if scripts names include a "." in their name as in this example: + + module.exports = { + scripts: { + "test": { + default: "cross-env CI=1 react-scripts test --env=jsdom", + watch: "react-scripts test --env=jsdom", + }, + "test.watch": "react-scripts test --env=jsdom", + }, + }; + +# How We Teach This + +I think 'importable scripts' accurately describes and portrays this feature. It should +be presented as a simple alternative to the current script structure, with the ability +of organization and comments. It is somewhat similar to other package.json field patterns +which list filenames as their value. The module structure is fairly similar, if not the same, +as nps: https://github.com/kentcdodds/nps. + +If accepted, the Yarn documentation would need to be updated to include the alternative +packages.json option as well as outline the implemented module structures as defined above. +As it is a more advanced and optional feature, it will not affect how yarn is currently used +or taught. + +This feature can be simply introduced via documentation as an optional alternative to specifying +scripts within the package.json file. User's will organically learn the feature as the need +for the feature arises. + +# Drawbacks + +1. Increase binary size. However small, there will be a slight increase in the binary size. +2. There will probably be some additional overhead for people having issues or understanding + how to implement the more advanced structures. +3. More time needed to create documentation. +4. As the package.json script field will now be used to specify a filename, we need to determine + if this will have any effect on other libraries or tools. One thing I noticed is that VS Code + does highlight that it as an 'Incorrect type. Expected Object'. None of my other tools seem to + complain. + +# Alternatives + +Similar functionality has already been implemented here: https://github.com/kentcdodds/nps. +While this package is useful, it requires using a separate cli for running scripts, or duplicating +script aliases in packages.json to point to nps. + +For the minimum amount of effort required, I think this feature should be implemented directly in +yarn allowing users to execute scripts with the cli they are comfortable using. + +Other designs for adding comments to scripts include putting the information in a separate file. +This would be useful, but it doesn't provide the level of functionality and organization that +this solution would. + +# Unresolved questions + +See item #4 under drawbacks \ No newline at end of file From 59bf9f36a225b94c8e913f18e3ec9f4f983054f5 Mon Sep 17 00:00:00 2001 From: cvanem Date: Fri, 17 May 2019 02:53:23 -0600 Subject: [PATCH 2/6] Formatting --- accepted/0000-importable-scripts.md | 115 +++++++++++----------------- 1 file changed, 43 insertions(+), 72 deletions(-) diff --git a/accepted/0000-importable-scripts.md b/accepted/0000-importable-scripts.md index 8d5a303..e0aadc8 100644 --- a/accepted/0000-importable-scripts.md +++ b/accepted/0000-importable-scripts.md @@ -4,18 +4,11 @@ # Summary -Allow scripts to be imported from a file as an alternative to the standard -object hash in the packages.json scripts field. Also define the basic -structure of the imported file. +Allow scripts to be imported from a file as an alternative to the standard object hash in the packages.json scripts field. Also define the basic structure of the imported file. # Motivation -The main motivation behind this is to allow scripts to be better organized -and documented in one place. This can be useful for large projects with -numerous scripts. It is also helpful for simplifying auto-generated -documentation and organizing scripts with comments or associated metadata. -This is not possible with the current package.json structure. If implemented, -this feature allows the following: +The main motivation behind this is to allow scripts to be better organized and documented in one place. This can be useful for large projects with numerous scripts. It is also helpful for simplifying auto-generated documentation and organizing scripts with comments or associated metadata. This is not possible with the current package.json structure. If implemented, this feature allows the following: 1. Ability to write comments next to an associated script. 2. Ability to include metadata with an associated script. @@ -23,57 +16,49 @@ this feature allows the following: # Detailed design -The intended design allows a filename to be specified in the packages.json -scripts field as an alternative to the default scripts object. This would then -be used to import scripts making them available to yarn via the current manifest -structure. +The intended design allows a filename to be specified in the packages.json scripts field as an alternative to the default scripts object. This would then be used to import scripts making them available to yarn via the current manifest structure. -The actual implementation would occur directly after the manifest is read, in -the tryManifest function. The new design would check the scripts field to -determine if it is an importable file. If it is, then the scripts would be -imported and effectively replace the filename in the manifest with the imported -scripts list. +The actual implementation would occur directly after the manifest is read, in the tryManifest function. The new design would check the scripts field to determine if it is an importable file. If it is, then the scripts would be imported and effectively replace the filename in the manifest with the imported scripts list. -For the basic structure used for the imported script module, I propose that it -is defined as follows: +The basic structure for the imported script module is proposed as follows: module.exports = { - scripts: { - ... - }, + scripts: { + ... + }, }; -For defining scripts, I propose supporting the following structures: +For defining scripts, the following structures are proposed: 1. Structure as a standard object hash inside of the scripts object. This would be identical to how scripts are currently specified in the - packages.json file with the additional ability of adding comments. + packages.json file with the additional ability of adding comments. - module.exports = { - scripts: { + module.exports = { + scripts: { "test": "cross-env CI=1 react-scripts test --env=jsdom", // Run tests "test:watch": "react-scripts test --env=jsdom", "build": "rollup -c", // Builds the library "start": "rollup -c -w", // Build and watch the library - }, - }; + }, + }; 2. Structure a script as an object with a 'script' key. This allows for additional metadata, other than comments, to be co-located with an associated script. When a script key is specified within an object, the object's remaining non object values are ignored. Remaining objects are imported imported as normal. - module.exports = { - scripts: { + module.exports = { + scripts: { "test": { - script: "cross-env CI=1 react-scripts test --env=jsdom", - description: "Run tests using react-scripts", + script: "cross-env CI=1 react-scripts test --env=jsdom", + description: "Run tests using react-scripts", }, "test:watch": "react-scripts test --env=jsdom", "build": "rollup -c", "start": "rollup -c -w", - }, - }; + }, + }; 3. Group scripts together using objects and allow a 'default' key. When scripts are grouped together, the script name or key will be generated from the object hierarchy. For example, @@ -83,46 +68,37 @@ For defining scripts, I propose supporting the following structures: it's parent's name. Therefore, in the below example, the test.default script would be exposed via the 'yarn test' command. - module.exports = { - scripts: { + module.exports = { + scripts: { "test": { - default: "cross-env CI=1 react-scripts test --env=jsdom", - watch: "react-scripts test --env=jsdom", + default: "cross-env CI=1 react-scripts test --env=jsdom", + watch: "react-scripts test --env=jsdom", }, "build": "rollup -c", "start": "rollup -c -w", - }, - }; + }, + }; -Checks and warnings will be put in place during script imports to flag any duplicate keys. -This can happen if scripts names include a "." in their name as in this example: +Checks and warnings will be put in place during script imports to flag any duplicate keys. This can happen if scripts names include a "." in their name as in this example: - module.exports = { - scripts: { - "test": { - default: "cross-env CI=1 react-scripts test --env=jsdom", - watch: "react-scripts test --env=jsdom", - }, - "test.watch": "react-scripts test --env=jsdom", - }, + module.exports = { + scripts: { + "test": { + default: "cross-env CI=1 react-scripts test --env=jsdom", + watch: "react-scripts test --env=jsdom", + }, + "test.watch": "react-scripts test --env=jsdom", + }, }; # How We Teach This -I think 'importable scripts' accurately describes and portrays this feature. It should -be presented as a simple alternative to the current script structure, with the ability -of organization and comments. It is somewhat similar to other package.json field patterns -which list filenames as their value. The module structure is fairly similar, if not the same, -as nps: https://github.com/kentcdodds/nps. +I think 'importable scripts' accurately describes and portrays this feature. It should be presented as a simple alternative to the current script structure, with the ability of organization and comments. It is somewhat similar to other package.json field patterns +which list filenames as their value. The module structure is fairly similar, if not the same, as nps: https://github.com/kentcdodds/nps. -If accepted, the Yarn documentation would need to be updated to include the alternative -packages.json option as well as outline the implemented module structures as defined above. -As it is a more advanced and optional feature, it will not affect how yarn is currently used -or taught. +If accepted, the Yarn documentation would need to be updated to include the alternative packages.json option as well as outline the implemented module structures as defined above. As it is a more advanced and optional feature, it will not affect how yarn is currently used or taught. -This feature can be simply introduced via documentation as an optional alternative to specifying -scripts within the package.json file. User's will organically learn the feature as the need -for the feature arises. +This feature can be simply introduced via documentation as an optional alternative to specifying scripts within the package.json file. User's will organically learn the feature as the need for it arises. # Drawbacks @@ -137,17 +113,12 @@ for the feature arises. # Alternatives -Similar functionality has already been implemented here: https://github.com/kentcdodds/nps. -While this package is useful, it requires using a separate cli for running scripts, or duplicating -script aliases in packages.json to point to nps. +Similar functionality has already been implemented here: https://github.com/kentcdodds/nps. While this package is useful, it requires using a separate cli for running scripts, or duplicating script aliases in packages.json to point to nps. -For the minimum amount of effort required, I think this feature should be implemented directly in -yarn allowing users to execute scripts with the cli they are comfortable using. +For the minimum amount of effort required, I think this feature should be implemented directly in yarn allowing users to execute scripts with the cli they are comfortable using. -Other designs for adding comments to scripts include putting the information in a separate file. -This would be useful, but it doesn't provide the level of functionality and organization that -this solution would. +Other designs for adding comments to scripts include putting the information in a separate file. This would be useful, but it doesn't provide the level of functionality and organization that this solution would. # Unresolved questions -See item #4 under drawbacks \ No newline at end of file +See item #4 under drawbacks From 30974c7d4f1d130778d8cda394bff9e3e5ef17ae Mon Sep 17 00:00:00 2001 From: cvanem Date: Fri, 17 May 2019 03:09:10 -0600 Subject: [PATCH 3/6] Formatting --- accepted/0000-importable-scripts.md | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/accepted/0000-importable-scripts.md b/accepted/0000-importable-scripts.md index e0aadc8..0837774 100644 --- a/accepted/0000-importable-scripts.md +++ b/accepted/0000-importable-scripts.md @@ -46,7 +46,7 @@ For defining scripts, the following structures are proposed: 2. Structure a script as an object with a 'script' key. This allows for additional metadata, other than comments, to be co-located with an associated script. When a script key is specified within an object, the object's remaining non object values - are ignored. Remaining objects are imported imported as normal. + are ignored. Remaining objects are imported as normal. module.exports = { scripts: { @@ -94,11 +94,11 @@ Checks and warnings will be put in place during script imports to flag any dupli # How We Teach This I think 'importable scripts' accurately describes and portrays this feature. It should be presented as a simple alternative to the current script structure, with the ability of organization and comments. It is somewhat similar to other package.json field patterns -which list filenames as their value. The module structure is fairly similar, if not the same, as nps: https://github.com/kentcdodds/nps. +which list filenames as their value. The module structure is similar to [nps](https://github.com/kentcdodds/nps). -If accepted, the Yarn documentation would need to be updated to include the alternative packages.json option as well as outline the implemented module structures as defined above. As it is a more advanced and optional feature, it will not affect how yarn is currently used or taught. +If accepted, the Yarn documentation would need to be updated to include the alternative packages.json option as well as outline the implemented module structures as defined above. As this is a more advanced and optional feature, it will not affect how yarn is currently used or taught. -This feature can be simply introduced via documentation as an optional alternative to specifying scripts within the package.json file. User's will organically learn the feature as the need for it arises. +This feature can be introduced via documentation as an optional alternative to including scripts within the package.json file. User's will organically learn the feature as their needs arise. # Drawbacks @@ -107,13 +107,12 @@ This feature can be simply introduced via documentation as an optional alternati how to implement the more advanced structures. 3. More time needed to create documentation. 4. As the package.json script field will now be used to specify a filename, we need to determine - if this will have any effect on other libraries or tools. One thing I noticed is that VS Code - does highlight that it as an 'Incorrect type. Expected Object'. None of my other tools seem to - complain. + if this will have any effect on other libraries or tools. One thing I noted is that VS Code + does highlight this field with an 'Incorrect type. Expected Object' warning when a filename is specified. # Alternatives -Similar functionality has already been implemented here: https://github.com/kentcdodds/nps. While this package is useful, it requires using a separate cli for running scripts, or duplicating script aliases in packages.json to point to nps. +Similar functionality has already been implemented in the [nps library](https://github.com/kentcdodds/nps). While this package is useful, it requires using a separate cli for running scripts, or duplicating script aliases in packages.json to point to nps. For the minimum amount of effort required, I think this feature should be implemented directly in yarn allowing users to execute scripts with the cli they are comfortable using. From 7042f8029091b52e07bd24ec985639ef66180275 Mon Sep 17 00:00:00 2001 From: cvanem Date: Fri, 17 May 2019 04:03:44 -0600 Subject: [PATCH 4/6] Add alternative --- accepted/0000-importable-scripts.md | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/accepted/0000-importable-scripts.md b/accepted/0000-importable-scripts.md index 0837774..997f4b8 100644 --- a/accepted/0000-importable-scripts.md +++ b/accepted/0000-importable-scripts.md @@ -112,6 +112,8 @@ This feature can be introduced via documentation as an optional alternative to i # Alternatives +We could specify the import filename in a reserved script key instead of using the scripts field. See the unresolved questions section for an example of this implementation. This solution has the added benefit of being able to use the default script definitions in conjuction with the imported list. + Similar functionality has already been implemented in the [nps library](https://github.com/kentcdodds/nps). While this package is useful, it requires using a separate cli for running scripts, or duplicating script aliases in packages.json to point to nps. For the minimum amount of effort required, I think this feature should be implemented directly in yarn allowing users to execute scripts with the cli they are comfortable using. @@ -120,4 +122,14 @@ Other designs for adding comments to scripts include putting the information in # Unresolved questions -See item #4 under drawbacks +See item #4 under drawbacks. If this drawback turns out to be an issue, we could specify the import script filename via a reserved 'import' script key instead: + + "scripts": { + "import": "package-scripts.js", + "test": "cross-env CI=1 react-scripts test --env=jsdom", + "test:watch": "react-scripts test --env=jsdom", + "build": "rollup -c", + "start": "rollup -c -w" + } + + From e7822609cb87e5ea1d7ff3e643bccae53da4b364 Mon Sep 17 00:00:00 2001 From: cvanem Date: Fri, 17 May 2019 04:06:40 -0600 Subject: [PATCH 5/6] Formatting --- accepted/0000-importable-scripts.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/accepted/0000-importable-scripts.md b/accepted/0000-importable-scripts.md index 997f4b8..05ebc92 100644 --- a/accepted/0000-importable-scripts.md +++ b/accepted/0000-importable-scripts.md @@ -8,7 +8,7 @@ Allow scripts to be imported from a file as an alternative to the standard objec # Motivation -The main motivation behind this is to allow scripts to be better organized and documented in one place. This can be useful for large projects with numerous scripts. It is also helpful for simplifying auto-generated documentation and organizing scripts with comments or associated metadata. This is not possible with the current package.json structure. If implemented, this feature allows the following: +The main motivation behind this is to better organize and document scripts in one place. This is useful for projects with numerous scripts. It is also helpful for simplifying auto-generated documentation and organizing scripts with comments or associated metadata. This is not possible with the current package.json structure. If implemented, this feature allows the following: 1. Ability to write comments next to an associated script. 2. Ability to include metadata with an associated script. From be7115a61f996a3f436deef135058b3910336629 Mon Sep 17 00:00:00 2001 From: Chris Van Emmerik Date: Sun, 19 May 2019 14:27:08 -0600 Subject: [PATCH 6/6] Add drawback --- accepted/0000-importable-scripts.md | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/accepted/0000-importable-scripts.md b/accepted/0000-importable-scripts.md index 05ebc92..2cdd4a3 100644 --- a/accepted/0000-importable-scripts.md +++ b/accepted/0000-importable-scripts.md @@ -103,12 +103,10 @@ This feature can be introduced via documentation as an optional alternative to i # Drawbacks 1. Increase binary size. However small, there will be a slight increase in the binary size. -2. There will probably be some additional overhead for people having issues or understanding - how to implement the more advanced structures. +2. There will probably be some additional overhead for people having issues or understanding how to implement the more advanced structures. 3. More time needed to create documentation. -4. As the package.json script field will now be used to specify a filename, we need to determine - if this will have any effect on other libraries or tools. One thing I noted is that VS Code - does highlight this field with an 'Incorrect type. Expected Object' warning when a filename is specified. +4. As the package.json script field will now be used to specify a filename, we need to determine if this will have any effect on other libraries or tools. One thing I noted is that VS Code does highlight this field with an 'Incorrect type. Expected Object' warning when a filename is specified. +5. When the file input is used, the scripts cannot be determined statically. The scripts must be determined by loading and executing javascript. # Alternatives