Skip to content

Enable users to import with import/require#1

Merged
includeDaniel merged 1 commit into
includeDaniel:mainfrom
includeVitor:main
Jul 29, 2023
Merged

Enable users to import with import/require#1
includeDaniel merged 1 commit into
includeDaniel:mainfrom
includeVitor:main

Conversation

@includeVitor

@includeVitor includeVitor commented Jul 29, 2023

Copy link
Copy Markdown
Contributor

Description

The users right now can't use "commonjs"/require with your package because you're using type="module" witch obligates all the users that download your package to use "module/import" witch is a issue, because in legacy projects you might need still use "commonjs"/require

What I changed

  • I removed type="module" from your package.json (with this change the users can use import/require syntax with your package)
  • I renamed rollup.config.js and jest.config.js files to rollup.config.mjs, jest.config.mjs (with this change you can still use import syntax inside those files)

Testing

  • Setup

    • Create a empty node app with npm init
    • Generate a new /dist with the command yarn build and copy /dist folder inside a node_modules/@includedaniel/fibonacci folder
  • Testing with commonjs/require syntax

    • Create a index.js file with the following content
       const fibonacci = require('@includedaniel/fibonacci')
       console.log(fibonacci(25))
    • With the command Node index.js you should get 75025
  • Testing with module/import syntax

    • Create a index.mjs file with the following content (if you want use index.js file you should add type="module" at this node app package.json)
       import fibonacci from '@includedaniel/fibonacci'
       console.log(fibonacci(25))
    • With the command Node index.mjs you should get 75025

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

Merging #1 (4d824cd) into main (e870c05) will not change coverage.
The diff coverage is n/a.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff            @@
##              main        #1   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           16        16           
  Branches         6         6           
=========================================
  Hits            16        16           

@includeDaniel
includeDaniel merged commit a412f11 into includeDaniel:main Jul 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants