Skip to content

fix(bazel): ng_package creates invalid typings reexport on windows - #27827

Closed
devversion wants to merge 4 commits into
angular:masterfrom
devversion:fix/ng_package-invalid-separators-windows
Closed

fix(bazel): ng_package creates invalid typings reexport on windows#27827
devversion wants to merge 4 commits into
angular:masterfrom
devversion:fix/ng_package-invalid-separators-windows

Conversation

@devversion

@devversion devversion commented Dec 25, 2018

Copy link
Copy Markdown
Member

Currently when building a package on Windows, the typings re-export for secondary entry-points is not valid TypeScript. Similarly the metadata and the "package.json" files use non-posix paths and cause inconsistency within the NPM package.

For example:

package.json

  "esm5": "./esm5\\core.js",
  "esm2015": "./esm2015\\core.js",

testing.d.ts (of the core package)

export * from './testing\testing';

Note: A test for the typings re-export is already part of the golden testing of the ng_package. Though, the golden testing seems to not run at the moment. We should try getting it work on Windows, and re-enable it on the CI in the future.

@devversion
fix(bazel): ng_package creates invalid typings reexport on windows
18b4ab1 
Currently when building a package on Windows, the typings re-export for secondary entry-points is not valid TypeScript. Similarly the metadata and the "package.json" files use non-posix paths and cause inconsistency within the NPM package.

For example:

_package.json_
```
  "esm5": "./esm5\\core.js",
  "esm2015": "./esm2015\\core.js",
```

_testing.d.t.s_ (of the `core` package)
```
export * from './testing\testing';
```
@devversion devversion added action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release area: bazel Issues related to the published `@angular/bazel` build rules labels Dec 25, 2018
@devversion
devversion requested a review from alexeagle December 25, 2018 15:01
@ngbot ngbot Bot added this to the needsTriage milestone Dec 25, 2018
@mary-poppins

mary-poppins commented Dec 25, 2018

Copy link
Copy Markdown

You can preview 18b4ab1 at https://pr27827-18b4ab1.ngbuilds.io/.

IgorMinar
IgorMinar reviewed Dec 25, 2018
View reviewed changes

@IgorMinar IgorMinar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a test please?

@mary-poppins

mary-poppins commented Dec 25, 2018

Copy link
Copy Markdown

You can preview f67e6b3 at https://pr27827-f67e6b3.ngbuilds.io/.

@devversion
devversion force-pushed the fix/ng_package-invalid-separators-windows branch from f67e6b3 to 042bd56 Compare December 25, 2018 20:05
@mary-poppins

mary-poppins commented Dec 25, 2018

Copy link
Copy Markdown

You can preview 042bd56 at https://pr27827-042bd56.ngbuilds.io/.

@devversion
devversion force-pushed the fix/ng_package-invalid-separators-windows branch from 042bd56 to 21d86e1 Compare December 25, 2018 20:32
@mary-poppins

This comment has been minimized.

Sign in to view
@devversion
devversion force-pushed the fix/ng_package-invalid-separators-windows branch 2 times, most recently from 59c9d63 to 9ca0bcc Compare December 25, 2018 20:40
@mary-poppins

This comment has been minimized.

Sign in to view
@mary-poppins

This comment has been minimized.

Sign in to view
@devversion
devversion force-pushed the fix/ng_package-invalid-separators-windows branch 2 times, most recently from c3af68d to 0ebf011 Compare December 25, 2018 21:21
@mary-poppins

mary-poppins commented Dec 25, 2018

Copy link
Copy Markdown

You can preview c3af68d at https://pr27827-c3af68d.ngbuilds.io/.

@mary-poppins

mary-poppins commented Dec 25, 2018

Copy link
Copy Markdown

You can preview 0ebf011 at https://pr27827-0ebf011.ngbuilds.io/.

@devversion
devversion force-pushed the fix/ng_package-invalid-separators-windows branch from 0ebf011 to 915e54d Compare December 25, 2018 21:29
@mary-poppins

mary-poppins commented Dec 25, 2018

Copy link
Copy Markdown

You can preview 915e54d at https://pr27827-915e54d.ngbuilds.io/.

@devversion
test(bazel): re-enable ng_package golden testing on ci
6c0c8c9 
* Enables the ng_package golden testing on the CI
* Fixes the ng_package golden testing for Windows
@devversion
devversion force-pushed the fix/ng_package-invalid-separators-windows branch from 915e54d to 6c0c8c9 Compare December 25, 2018 22:14
@devversion

devversion commented Dec 25, 2018 •
edited
Loading

Copy link
Copy Markdown
Member Author

I've made a lot of changes, and this PR turned out way bigger than I initially planned. Closing this as I'm going to create a new PR which is easier to review.

@devversion devversion closed this Dec 25, 2018
@devversion devversion mentioned this pull request Dec 25, 2018
Closed
@angular-automatic-lock-bot

angular-automatic-lock-bot Bot commented Sep 14, 2019

Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@alexeagle alexeagle Awaiting requested review from alexeagle
1 more reviewer
@IgorMinar IgorMinar IgorMinar left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

action: review The PR is still awaiting reviews from at least one requested reviewer area: bazel Issues related to the published `@angular/bazel` build rules cla: yes target: patch This PR is targeted for the next patch release

Projects

None yet

Milestone

needsTriage

Development

Successfully merging this pull request may close these issues.

4 participants

@devversion @mary-poppins @IgorMinar @googlebot