-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
feat: angular package format #1474
Conversation
Type annotation on fields with forwardRef lead to 'Cannot access X before initialization' error in es2015 build. This change also fixes circular dependency warning.
Prevents "Maximum call stack size exceeded" when building with "flatModuleOutFile". angular#25226
Fixes "Cannot access NbLayoutComponent before initialization". When building with ES2015 target, "forwardRef" in header breaks. angular#30106.
angular#20931
angular#20931
angular#20931
They will be inserted during the build
Enables more aggressive webpack optimizations
…t/ng8-es2015-bundle
Codecov Report
@@ Coverage Diff @@
## master #1474 +/- ##
==========================================
+ Coverage 82.95% 83.29% +0.34%
==========================================
Files 240 230 -10
Lines 7332 7489 +157
Branches 659 659
==========================================
+ Hits 6082 6238 +156
- Misses 1060 1061 +1
Partials 190 190
|
scripts/ci/packages-smoke.sh
Outdated
@@ -17,8 +17,10 @@ packages_smoke() { | |||
cp -r ../nebular/src/.lib/* node_modules/@nebular | |||
|
|||
echo "Verifying application build" | |||
npm run build --prod --aot | |||
npm run build -- --prod --aot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty sure we don't need --aot flag here because it's already listed as aot: true in angular.json for production environment. Could you please, remove it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed in 3ef9057
DEV_DOCS.md
Outdated
@@ -27,6 +27,7 @@ | |||
- add the package into bump-version.ts which bumps package version and its dependencies | |||
- add the package external dependencies into rollup-config.ts which gives rollup capability build package correctly | |||
- add the package into bundle.ts which build umd modules for our packages | |||
- add the package into `JS_PACKAGES` in config.ts which used to add es2015 bundles for our packages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please mention where that config.ts lives? 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added in 9d0cebd
scripts/gulp/tasks/change-prefix.ts
Outdated
@@ -6,7 +6,7 @@ import * as replace from 'gulp-replace'; | |||
import * as minimist from 'minimist'; | |||
import { capitalize, dasherize } from '@angular-devkit/core/src/utils/strings'; | |||
|
|||
import { replaceScssWithCss } from './copy-sources'; | |||
// import { replaceScssWithCss } from './copy-sources'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that line has to be removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed in c15e9b6
scripts/gulp/tasks/change-prefix.ts
Outdated
@@ -218,7 +218,7 @@ task('change-prefix', ['copy-build-dir-and-rename', 'generate-ts-config', 'patch | |||
|
|||
return stream | |||
.pipe(dest(BUILD_DIR)) | |||
.on('end', replaceScssWithCss); | |||
// .on('end', replaceScssWithCss); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that line has to be removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed in c15e9b6
I personally like the implementation 😄 👍 |
LGTM |
Thanks 🙂 |
Please read and mark the following check list before creating a pull request:
Short description of what this resolves:
Adds
esm2015
,fesm2015
,fesm5
into packages distributives.BREAKING CHANGE:
Everything not mentioned in packages
public_api.ts
now private. Direct imports from (@nebular/theme/*
) will stop working.