Skip to content
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(module): support src as a function in addModule #4956

Merged
merged 1 commit into from
Feb 5, 2019
Merged

feat(module): support src as a function in addModule #4956

merged 1 commit into from
Feb 5, 2019

Conversation

ricardogobbosouza
Copy link
Contributor

@ricardogobbosouza ricardogobbosouza commented Feb 5, 2019

Types of changes

  • Bug fix (a non-breaking change which fixes an issue)
  • New feature (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

While developing a module I came across a problem in its coverage.

If you are analyzing the modules such as pwa, moment-module, nuxt-i18n the coverage is made on top of the test folder and not the lib or src, this is why nuxt solves the module using esm by default, so the module is not is covered.

Today to test the modules we created a nuxt.config.js like this:

modules: [
  '@@'
]

and this has no coverage.

With this correction we can create like this:

modules: [
  require('../../')
]

and this has coverage

If we want to create the module using ESM and with coverage we have to add new configurations in jest and babel:

  "babel": {
    "presets": [
      [
        "env",
        {
          "modules": false
        }
      ]
    ],
    "env": {
      "test": {
        "presets": [
          [
            "env",
            {
              "targets": {
                "node": "current"
              }
            }
          ]
        ]
      }
    }
  },
  "jest": {
    "testEnvironment": "node",
    "collectCoverage": true,
    "collectCoverageFrom": [
      "lib/**/*.js"
    ],
    "transform": {
      "^.+\\.js$": "babel-jest"
    }
  }

and add the dependencies babel-jest and babel-preset-env

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly. (PR: #)
  • I have added tests to cover my changes (if not applicable, please state why)
  • All new and existing tests are passing.

@manniL
Copy link
Member

manniL commented Feb 5, 2019

Does that fix #4119 😱

@ricardogobbosouza
Copy link
Contributor Author

ricardogobbosouza commented Feb 5, 2019

@manniL yeah! ::smiley:
I didn't realize that I had issue

@pi0
Copy link
Member

pi0 commented Feb 5, 2019

Fantastic investigation and improvement for module coverage. For ecosystem consistency, I suggest not documenting this to prevent module incompatibility for 2.x users prior to this version.

@ricardogobbosouza Would you please also open a PR on module template to reflect changes to the default test suit?

@nuxt nuxt deleted a comment from codecov-io Feb 5, 2019
@ricardogobbosouza
Copy link
Contributor Author

@pi0 I will suggest improvements in module-template

@pi0 pi0 changed the title fix(core): enable src with a function in addModule fix(module): enable src with a function in addModule Feb 5, 2019
@pi0 pi0 changed the title fix(module): enable src with a function in addModule feat(module): support src as a function in addModule Feb 5, 2019
@pi0 pi0 merged commit 1e9eb4b into nuxt:dev Feb 5, 2019
@ricardogobbosouza
Copy link
Contributor Author

ricardogobbosouza commented Feb 5, 2019

Fantastic investigation and improvement for module coverage. For ecosystem consistency, I suggest not documenting this to prevent module incompatibility for 2.x users prior to this version.

@ricardogobbosouza Would you please also open a PR on module template to reflect changes to the default test suit?

@pi0 PR on module template
nuxt-community/module-template#9

@pi0 pi0 mentioned this pull request Mar 14, 2019
@danielroe danielroe added the 2.x label Jan 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants