Skip to content

chore: use import instead of require for bundling diff command#3847

Merged
fahslaj merged 1 commit intolerna:mainfrom
amorscher:ts-fixes-use-import
Oct 17, 2023
Merged

chore: use import instead of require for bundling diff command#3847
fahslaj merged 1 commit intolerna:mainfrom
amorscher:ts-fixes-use-import

Conversation

@amorscher
Copy link
Copy Markdown
Contributor

@amorscher amorscher commented Sep 23, 2023 •
edited
Loading

  • ensures that typechecks are executed for diff command during build

Description

using require does not execute type checks during npx nx run-many -t build --parallel=3

Motivation and Context

Ensure typechecking during build for libs as well. Remove warnings/errors from eslint regarding require statements

How Has This Been Tested?

All unit tests and integration tests still pass.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (change that has absolutely no effect on users)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@amorscher amorscher marked this pull request as draft September 23, 2023 07:51
@amorscher
Copy link
Copy Markdown
Contributor Author

amorscher commented Sep 24, 2023 •
edited
Loading

@fahslaj @JamesHenry what do you think about this approach?

I recognized that during build no typechecks are executed for libs/command libraries. I know for tests typechecks are applied but not during build.
Replacing require/module.export statements with import/export enables typechecking during build.

@fahslaj
Copy link
Copy Markdown
Contributor

fahslaj commented Oct 14, 2023

@amorscher I like it. When changing how these commands and functions are exported, we should ensure that they can be imported by end users in the same way as before. For example:

const diff = require('lerna/commands/diff');
const getLastCommit = require('lerna/commands/diff/lib/get-last-commit');

I just tested this branch locally and this appears to be the case with diff after the change, so we should be good.

@amorscher
Copy link
Copy Markdown
Contributor Author

amorscher commented Oct 16, 2023

@fahslaj Ok good. Thx for having a look!
So I will apply this to other commands as well. I will do this command after command.

@amorscher amorscher closed this Oct 16, 2023
@amorscher amorscher reopened this Oct 16, 2023
@amorscher amorscher marked this pull request as ready for review October 16, 2023 21:25
@amorscher amorscher changed the title chore: use import instead of require for bundling commands chore: use import instead of require for bundling diff command Oct 16, 2023
@amorscher
Copy link
Copy Markdown
Contributor Author

amorscher commented Oct 16, 2023

I will update the commit message to make it clear that only diff command is changed for now

@fahslaj fahslaj enabled auto-merge (squash) October 17, 2023 16:08
@amorscher @fahslaj
chore: use import instead of require for bundling commands
915e476 
 - ensures that typechecks are executed during build command
@fahslaj fahslaj force-pushed the ts-fixes-use-import branch from a1891c5 to 915e476 Compare October 17, 2023 16:08
@fahslaj fahslaj merged commit af23df5 into lerna:main Oct 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JamesHenry JamesHenry JamesHenry approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@amorscher @fahslaj @JamesHenry