search

amclin / aem-packager

A node plugin that creates AEM packages installable through the Adobe Experience Manager package manager.
MIT License
16 stars 4 forks source link

groupId doesn't work in NPM 7 and 8 #395

Closed amclin closed 2 years ago

amclin commented 2 years ago

Describe the bug NPM package setting configurations don't work with Node 8. GroupId is no longer passed through from package.json to the generated package

To Reproduce Steps to reproduce the behavior:

  1. Set the groupId in package.json
  2. Run aem-packager
  3. Observe the contents of the generated package

Expected behavior GroupId is passed through

Screenshots and Logs

Environment (please complete the following information):

Additional context Starting in NPM 8 (possibly NPM 7? but that's not a LTS version) the variables in process.env that are namespaced npm_package_ are no longer available. The only one passed through is npm_package_version. Other expected ones like npm_package_aem-packager_groupId are no longer available, presumably for security reasons. See #385

Needs some research to identify the preferred method of passing these variables in newer NPM and if this breaks the various methods currently supported for providing configurations to aem-packager https://github.com/npm/cli/issues/2609 https://github.com/npm/rfcs/blob/main/implemented/0021-reduce-lifecycle-script-environment.md

amclin commented 2 years ago

From some initial reading, NPM 7/8 provides two solutions for this:

  1. Using the npm_package_config_ namespace instead of npm_package_. From how the documentation is worded, this seems like a special case for backwards compatibility, implying it may not be supported in the future
  2. Provides the path to the package.json as a variable in process.env so the consuming script can do its own loading and parsing. This seems a bit messy to me, as now the consuming script has to do the heavy lifting of reading from the filesystem and parsing JSON, but I get why they want to reduce the size of the overall object being passed around. This might be the more future-proof approach.

Approach #2 could allow for supporting the current configurations on NPM 8 without making a breaking change in this package, but would still require adding version detection and continuing to supporting logic for NPM 4-6. Approach #1 could be done in a non-breaking way, would also introduce version detection logic, but the old config locations would never work in NPM 7 and above. Having two potential structures for configuration will cause consumer confusion, and thus potentially far more bug reports for as long as both configuration specifications are supported.

Need to decide if this will be implemented as a breaking change.

amclin commented 2 years ago

:tada: This issue has been resolved in version 3.1.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: