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

fix: update deps for fix bundle compilation #273

Merged
merged 2 commits into from
Dec 20, 2021
Merged

Conversation

Delagen
Copy link
Collaborator

@Delagen Delagen commented Dec 15, 2021

@Delagen
Copy link
Collaborator Author

Delagen commented Dec 15, 2021

@urish Try this to resolve 6.0.0 release issue

@urish
Copy link
Owner

urish commented Dec 15, 2021

Thanks, have you tested this?

Also, husky 6 had some breaking changes, so if you upgrade it, please update the configuration accordingly

@Delagen
Copy link
Collaborator Author

Delagen commented Dec 15, 2021

Thanks. Updated MR

@urish
Copy link
Owner

urish commented Dec 15, 2021

Thanks for the update. Have you tested that this actually fixes the issue in #272?

@Delagen
Copy link
Collaborator Author

Delagen commented Dec 15, 2021

added "postinstall": "husky install" to support yarn.
When dist compiled, section scripts removed, so it's safe.

@urish
Copy link
Owner

urish commented Dec 15, 2021

I'm not sure I explained myself correctly - do we have any evidence that this change solves the issue in #272?

specifically, this:

Error: export 'MomentModule' (imported as 'MomentModule') was not found in 'ngx-moment' (module has no exports)

@Delagen
Copy link
Collaborator Author

Delagen commented Dec 20, 2021

Error cause that bundle in fesm2020 in npm package consist only from root wrapper and does not contain any more code.
Seems it was error in early version of 13 compiler

Thats the content of packaged bundle

"use strict";
/**
 * Generated bundle index. Do not edit.
 */
var __createBinding = (this && this.__createBinding) || (Object.create ? (function(o, m, k, k2) {
    if (k2 === undefined) k2 = k;
    Object.defineProperty(o, k2, { enumerable: true, get: function() { return m[k]; } });
}) : (function(o, m, k, k2) {
    if (k2 === undefined) k2 = k;
    o[k2] = m[k];
}));
var __exportStar = (this && this.__exportStar) || function(m, exports) {
    for (var p in m) if (p !== "default" && !Object.prototype.hasOwnProperty.call(exports, p)) __createBinding(exports, m, p);
};
Object.defineProperty(exports, "__esModule", { value: true });
__exportStar(require("./index"), exports);
//# sourceMappingURL=ngx-moment.mjs.map

As you see no any usefull code
fesm2015 the same

@urish
Copy link
Owner

urish commented Dec 20, 2021

Got it, thanks for explaining!

One more thing: I see the commit includes both "packages-lock.json" and "yarn.lock". Let's keep remove one of them (I don't have a strong opinion which), otherwise it's easy to get them out of sync.

@Delagen
Copy link
Collaborator Author

Delagen commented Dec 20, 2021

NPM 8 keep it sync, I think better to have NPM instead of yarn, I have seen few projects that need yarn really.

@urish
Copy link
Owner

urish commented Dec 20, 2021

So can you please remove yarn.lock? Not everyone has npm 8 yet

@Delagen
Copy link
Collaborator Author

Delagen commented Dec 20, 2021

Done

@urish
Copy link
Owner

urish commented Dec 20, 2021

Thank you!

@urish urish merged commit 2dea39d into urish:master Dec 20, 2021
@Delagen Delagen deleted the update-deps branch December 20, 2021 12:12
@urish
Copy link
Owner

urish commented Dec 20, 2021

released as 6.0.1

@urish
Copy link
Owner

urish commented Dec 20, 2021

Thanks again for looking into this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants