Esm modules fix
Describe your change
This is an initial attempt to fis the module formats of the libraries published from this repo. I think that the current issues with the published packages are:
- despite using ESM2022 module format the tpye of the package is not marked as
module- this means that node does not know that it should treat the files as mjs rather than cjs - module resolution is not set to
NodeNextwhich means that we do not specify extensions on our imports and esm consuming apps will not be able to consume the files (unless bundler magic deals with this which I think webpack and angular build systems do)
To fix these issues this PR:
- marks published libraries as
"type": "module" - sets module resolution to "NodeNext"
- adds extensions to imports
Related Issue
fixes #1609
Contributor License Agreement
- [x] I acknowledge that a contributor license agreement is required and that I have one in place or will seek to put one in place ASAP.
Review Checklist
- [ ] Issue: If a change was made to the FDC3 Standard, was an issue linked above?
- [ ] CHANGELOG: Is a CHANGELOG.md entry included?
- [ ] API changes: Does this PR include changes to any of the FDC3 APIs (
DesktopAgent,Channel,PrivateChannel,Listener,Bridging)?- [ ] Docs & Sources: If yes, were both documentation (/docs) and sources updated?
JSDoc comments on interfaces and types should be matched to the main documentation in /docs - [ ] Conformance tests: If yes, are conformance test definitions (/toolbox/fdc3-conformance) still correct and complete?
Conformance test definitions should cover all required aspects of an FDC3 Desktop Agent implementation, which are usually marked with a MUST keyword, and optional features (SHOULD or MAY) where the format of those features is defined - [ ] Schemas: If yes, were changes applied to the Bridging and FDC3 for Web protocol schemas?
The Web Connection protocol and Desktop Agent Communication Protocol schemas must be able to support all necessary aspects of the Desktop Agent API, while Bridging must support those aspects necessary for Desktop Agents to communicate with each other- [ ] If yes, was code generation (
npm run build) run and the results checked in?
Generated code will be found at/src/api/BrowserTypes.tsand/or/src/bridging/BridgingTypes.ts
- [ ] If yes, was code generation (
- [ ] Docs & Sources: If yes, were both documentation (/docs) and sources updated?
- [ ] Context types: Were new Context type schemas created or modified in this PR?
- [ ] Were the field type conventions adhered to?
- [ ] Was the
BaseContextschema applied viaallOf(as it is in existing types)? - [ ] Was a
titleanddescriptionprovided for all properties defined in the schema? - [ ] Was at least one example provided?
- [ ] Was code generation (
npm run build) run and the results checked in?
Generated code will be found at/src/context/ContextTypes.ts
- [ ] Intents: Were new Intents created in this PR?
- [ ] Were the intent name prefixes and other naming conventions & characteristics adhered to?
- [ ] Was the new intent added to the list in the Intents Overview?
THIS SOFTWARE IS CONTRIBUTED SUBJECT TO THE TERMS OF THE FINOS CORPORATE CONTRIBUTOR LICENSE AGREEMENT.
THIS SOFTWARE IS LICENSED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE AND ANY WARRANTY OF NON-INFRINGEMENT, ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. THIS SOFTWARE MAY BE REDISTRIBUTED TO OTHERS ONLY BY EFFECTIVELY USING THIS OR ANOTHER EQUIVALENT DISCLAIMER IN ADDITION TO ANY OTHER REQUIRED LICENSE TERMS.
Deploy Preview for fdc3 ready!
| Name | Link |
|---|---|
| Latest commit | fa6e04d3d411fdbcaabb38b12849b87fdbae49ba |
| Latest deploy log | https://app.netlify.com/projects/fdc3/deploys/6909cadf0d04b2000897b778 |
| Deploy Preview | https://deploy-preview-1677.preview-fdc3.finos.org |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify project configuration.
Currently this PR successfully builds all packages but it does not run the demo app when npm run dev is executed. There are issues with the demo web server.
The testing and fdc3-conformance packages are not converted to ESM as there were issues there with some imports (AJV seems to be used in a non standard way that ESM was not happy with) not working and other issues. As these are not publiushed I think this is OK.
I still need to get the demo server working correctly but I ran out of time today.
I imagine this PR will be a merge nightmare as it has touched so many files.... I don't know what the best way to deal with that will be.
@Roaders
I imagine this PR will be a merge nightmare as it has touched so many files.... I don't know what the best way to deal with that will be.
I don't think it will be that bad as most of the changes are in the imports into files. It would pick up conflicts with new additions, but won't with many other PRs. Regardless, we should try and get this done and merged ASAP.
@hughtroeger has offered to try and get this tested as a solution for @enewsome's issue #1609
However, the tests / cucumber see quite unhappy with the change and need further work. The SAST scan failing is down to the rename of a script file that needs to be in .semgrepignore here: https://github.com/finos/FDC3/blob/16100f51943512e944fb2d7ee767e31a7770d536/.semgrepignore#L4-L5 (.js extension changes to .cjs)
yes I think that the tests will be an issue. Converting to ESM is usually fairly painless other than things liks jest / karma / webpack / ts-node. I have already replaced ts-node with tsx as it's much happier with esm but have no idea how hard it will be to convince the testing framework to work - especially as I have no experience with this test runner.
I've spent a bit more time on this today and it's not good news I am afraid.... Apparently everything now builds correctly with no errors.... but unfortunately when I try to run the demo app I get runtime errors about not being able to find certain imports so more work is obviously required there. The other issue that could take much longer to fix is the tests as @kriswest mentioned before. Cucumber does apparently support esm but I don't know if nyc does (I have never come across nyc before and have not done anything with cucumber for a very long time). After a quick google it seems that nyc is supposed to support ESM through node's own esm loader. I have not had much luck with that which is why I have replaces ts-node with tsx.
This could end up being a much bigger job that we first thought.
I have confirmed that the fixes that I have put into this PR so far have fixed my issues when trying to run vitest in my local app. This is the case where my lib depends on @morgan-stanley/fdc3-web which depends on @finos/fdc3 and fails due to there being no file extensions on any of the imports.
The fix is good.... we "just" need to get the rest of the tooling in the mono repo working in an esm environment.
The fix is good.... we "just" need to get the rest of the tooling in the mono repo working in an esm environment.
Many thanks for your work on this issue @Roaders! @hughtroeger and @enewsome it would be worth confirming if this branch resolves some of your issues...
@Roaders which tooling most needs looking at?
- You mentioned nyc,
- semgrep needs its ignore file updating (s2tQuicktypeUtil.cjs change extension so is not being ignored),
- CVE scanning with audit.js is broken on main (@robmoffat can we get FINOS' help with that - could be an account issue: https://github.com/finos/FDC3/actions/runs/19058409074/job/54433286532)
- Lots seems broken in the tests - so I assume this is the area most in need of looking at...
Also want to express thanks for working on this @Roaders. We will test the changes in this PR and get back to you shortly.
I'm afraid that to test it you're going to have to check it out locally and build it locally then install each of the built packages one by one in your app. This worked for me. It's the tests that are not working in the PR.
@Roaders understood, I have built this branch and npm packed each of the individual packages and handed them over to @enewsome for testing. Thanks!