change ts_proto_library to default es5 wrapping #1593
Conversation
This change wraps es5 sources generated with `ts_proto_library` to be wrapped in "default" mode. The "default" mode behaves like UMD, supporting both AMD and CommonJS. This change is necessary to support Node, as Node does not use AMD. See #562
CooperBills
requested review from
alexeagle,
gregmagolan and
soldair
as code owners
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
|
@googlebot I fixed it. |
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
| it('should work in node', () => { | ||
| expect(TestMessage.verify({ | ||
| testField: 'Hello', | ||
| })).toBeFalsy(); // verify returns an error if failed |
There was a problem hiding this comment.
should there be a success case too? or is it enough to assert that it doesn't throw an error?
There was a problem hiding this comment.
This test case is really just to run some code - I could change it to expect(TestMessage).toBeTruthy(); and get the verification needed for this PR, would you prefer the simpler test case?
|
Thanks! |
This change wraps es5 sources generated with
ts_proto_libraryto be wrapped in "default" mode. The "default" mode behaves like UMD, supporting both AMD and CommonJS. This change is necessary to support Node, as Node does not use AMD. Fixes #562PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
When using
ts_proto_library, the application fails at runtime withReferenceError: define is not definedIssue Number: #562
What is the new behavior?
Works in Node -
ts_proto_librarywill now emit UMD modules instead of AMD.Does this PR introduce a breaking change?
Other information