Skip to content
This repository has been archived by the owner on Nov 29, 2023. It is now read-only.

Fix import error for enketo-transformer/web #184

Merged
merged 1 commit into from
Aug 2, 2023

Conversation

raphaelmerx
Copy link
Contributor

Fixes enketo/enketo#759

Re-ordering of types to be before default in the web exports.

from https://nodejs.org/api/packages.html#conditional-exports:

"default" - the generic fallback that always matches. Can be a CommonJS or ES module file. This condition should always come last.

from https://nodejs.org/api/packages.html#conditional-exports
> "default" - the generic fallback that always matches. Can be a CommonJS or ES module file. This condition should always come last.
Copy link
Contributor

@eyelidlessness eyelidlessness left a comment

Choose a reason for hiding this comment

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

I had also recently noticed the same note in the Node docs, and intended to make the same change. Thanks for getting to it before I did!

@eyelidlessness
Copy link
Contributor

I'm holding off merging this for a bit, because I saw another issue while installing to test the change in the web demo. It's unrelated to this change (probably related to #181), but it's preventing manual validation there. I'll likely test the change in Core/Express so the unrelated issue doesn't block this.

@eyelidlessness
Copy link
Contributor

I've verified this locally in Core, which will be the basis for a PR there to switch to client rendering in dev mode soon. Local verification has some caveats (local NPM dependencies don't get packaged the way they do when publishing), but I was able to observe a difference in behavior depending on the order.

Some other notes for my future reference (and may be helpful to anyone else adopting the web transform in a project with a similar setup):

  • TypeScript (or at least the TypeScript VSCode language server) does not seem to care about this ordering (it depends on typesVersions instead, because of course it does). But other tooling does, notably ESLint and...
  • ... typescript-eslint, which was fussy in a whole bunch of other ways. To get ESLint working, all of the following changes were necessary:
    • Install all of these as dev dependencies:

      • @typescript-eslint/eslint-plugin
      • @typescript-eslint/parser
      • eslint-import-resolver-typescript
      • typescript
    • Add all of this to .eslintrc.json (I guess we could skip some of the file extensions, but I think the list is [currently] exhaustive, included as a frame of reference for anyone else who may be interested):

       {
       …
      -  "plugins": ["chai-friendly", "jsdoc", "prettier", "unicorn"],
      +  "plugins": ["@typescript-eslint", "chai-friendly", "jsdoc", "prettier", "unicorn"],
      +  "parser": "@typescript-eslint/parser",
       …
         "settings": {
      +    "import/parsers": {
      +        "@typescript-eslint/parser": [
      +            ".cjs",
      +            ".cts",
      +            ".js",
      +            ".mjs",
      +            ".mts",
      +            ".ts",
      +            ".tsx"
      +        ]
      +    },
      +    "import/resolver": {
      +        "node": true,
      +        "typescript": true
      +    },
       …
         },
       …
         "overrides": {
      +    {
      +        "files": ["./app.js"],
      +        "rules": {
      +            "import/no-extraneous-dependencies": [
      +                "error",
      +                { "devDependencies": true }
      +            ]
      +        }
      +    },
       …
       }
  • The build (currently) needs to have libxslt added to external. This is necessary because we currently use a dynamic import to load it in the Node build. We may be able to conditionally compile it out, to make this unnecessary for downstream users in the future.

@eyelidlessness eyelidlessness merged commit 1fc23df into enketo:master Aug 2, 2023
5 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error importing enketo-transform/web: Module not found: Error: Default condition should be last one
2 participants