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

Regression: wrong module versions get packaged when wrongly deduped #672

Closed
trieloff opened this issue Mar 20, 2019 · 5 comments · Fixed by #980
Closed

Regression: wrong module versions get packaged when wrongly deduped #672

trieloff opened this issue Mar 20, 2019 · 5 comments · Fixed by #980
Assignees
Labels
bug Something isn't working released

Comments

@trieloff
Copy link
Contributor

On the dev site, @filmaj experienced a regression of #645 (ajv 5x ends up in html.zip instead of ajv 6x).

I haven't been able to reproduce it using the helix-cli integration tests yet, so we need to check against the actual dev site setup.

@filmaj can you describe your development setup?

  • hlx version
  • installed where
  • installed how
@trieloff trieloff added the bug Something isn't working label Mar 20, 2019
@filmaj
Copy link
Contributor

filmaj commented Mar 20, 2019

hlx version: 0.13.11-pre.8
installed via package.json: {dependencies:{"@adobe/helix-cli":"0.13.11-pre.8"}}
installed via npm install

@filmaj
Copy link
Contributor

filmaj commented Mar 20, 2019

Then from https://github.com/adobe/developer.adobe.com e.g. cd branch, run hlx clean && hlx build && hlx package, then unzip .hlx/build/html.zip and run grep version .hlx/build/node_modules/ajv/package.json. Expectation: 6.0.0 or newer. Actual: 5.5.2.

@tripodsan
Copy link
Contributor

tripodsan commented Mar 21, 2019

the problem is that the current module-pack algorithm, include the module that is most top level module if there are duplicates.
when installing helix-cli globally (or via installer), it also respects the devDependencies which include ajv@6. but when installing locally, it doesn't and the ajv in ./node_modules/@adobe/helix-cli/node_modules/ajv is the one from request, since it doesn't need the devDependencies one.

the quick workaround is to add ajv@6 as normal dependency to helix-cli, in order to pull it up into it node_modules.

the correct fix is probably, to detect the duplication, and try to keep the [email protected] local to request.

@tripodsan
Copy link
Contributor

temporary workaound applied in @adobe/[email protected]

@tripodsan tripodsan self-assigned this Mar 21, 2019
@tripodsan tripodsan changed the title Regression for #645 Regression: wrong module versions get packaged when wrongly deduped Mar 22, 2019
tripodsan added a commit that referenced this issue Mar 22, 2019
tripodsan added a commit that referenced this issue Jun 11, 2019
@tripodsan tripodsan mentioned this issue Jun 11, 2019
3 tasks
tripodsan added a commit that referenced this issue Jun 11, 2019
tripodsan added a commit that referenced this issue Jun 12, 2019
tripodsan added a commit that referenced this issue Jun 12, 2019
tripodsan added a commit that referenced this issue Jun 12, 2019
tripodsan added a commit that referenced this issue Jun 12, 2019
tripodsan added a commit that referenced this issue Jun 13, 2019
tripodsan added a commit that referenced this issue Jun 13, 2019
tripodsan added a commit that referenced this issue Jun 13, 2019
tripodsan added a commit that referenced this issue Jun 13, 2019
tripodsan added a commit that referenced this issue Jun 13, 2019
trieloff pushed a commit that referenced this issue Jun 13, 2019
## [4.3.1](v4.3.0...v4.3.1) (2019-06-13)

### Bug Fixes

* **package:** remove unused dependencies ([bc13466](bc13466))
* **packager:** implement better way to bundle actions ([c2adb80](c2adb80)), closes [#966](#966) [#672](#672) [#589](#589)
@adobe-bot
Copy link
Collaborator

🎉 This issue has been resolved in version 4.3.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants