Mdx: unused code and redefined variables, and set initial value

[Himenon]

Subject of the issue

I would like to refactor the code below. Because I used TypeScript, and concerned about the error.

• Redefined variables
  
  
  
  • https://github.com/mdx-js/mdx/blob/master/packages/mdx/index.js#L130 <-- want to remove
  
  
  • https://github.com/mdx-js/mdx/blob/master/packages/mdx/index.js#L134
  
  
• unused code
  
  
  
  • https://github.com/mdx-js/mdx/blob/master/packages/mdx/test/index.test.js#L3
  
  
  • https://github.com/mdx-js/mdx/blob/master/packages/tag/test/mdx-tag.test.js#L1
  
  
  • https://github.com/mdx-js/mdx/blob/master/packages/tag/test/mdx-provider.test.js#L1
  
  
  • https://github.com/mdx-js/mdx/blob/master/packages/runtime/test/index.test.js#L3
  
  
• Specifying initial values for optional arguments
  
  
  
  • https://github.com/mdx-js/mdx/blob/master/packages/tag/src/mdx-provider.js#L5
  
  

Expected behaviour

• Redefined variables

# https://github.com/mdx-js/mdx/blob/master/packages/mdx/index.js
- compile.sync = sync

module.exports = compile
exports = compile
exports.sync = sync

• unused code
  
  
  
  • remove
  
  
• Specifying initial values for optional arguments

# https://github.com/mdx-js/mdx/blob/master/packages/tag/src/mdx-provider.js#L5
- export const withMDXComponents = Component => ({ components, ...props }) => (
+ export const withMDXComponents = Component => ({ components = undefined, ...props }) => (

[silvenon]

I never used TypeScript, I have only used Flow, so I'm not the best person to handle this conversation. I'll just add some notes.

compile.sync is part of the API, is there a comment or something that we could add to ignore the TypeScript error? Also, making the argument default to undefined is odd because it already is undefined if it's using the default value. Is the another workaround?

[Himenon]

Thank you for your reply.

compile.sync is part of the API, is there a comment or something that we could add to ignore the TypeScript error?

I think that compile.sync and exports.sync = sync have the same meaning. References on the import or require side will see the one defined after compile.sync and exports.sync = sync, so the result will be the same.

It is simply an error that the previously defined one is overwritten, so if either compile.sync or exports.sync = sync remains, it will fix it.

Also, making the argument default to undefined is odd because it already is undefined if it's using the default value. Is the another workaround?

Sorry, my environment is too strict. Even with the current situation, the result is the same, so there is no problem even if it is as it is.

[ChristopherBiscardi]

@Himenon What is your environment? How can we reproduce seeing these issues?

[silvenon]

Oh, I see, setting compile.sync does seem redundant.

Concerning unused variables, you can wait for #300 to get merged, then add an ESLint rule no-unused-vars, which only applies to code in packages (you can set that in the overrides part).

So you can put all of that in the same PR.

[Himenon]

@ChristopherBiscardi

If you are using VSCode, you can confirm by attaching @ts-check to the first line of the js file. This is the easiest way to reproduce.

https://code.visualstudio.com/docs/languages/javascript#_type-checking

If you want to check with cli, download the next branch locally.

git clone -b chore/detect-typescript-error https://github.com/Himenon/mdx.git
cd mdx
yarn install

Then execute the following command. You can check the error.

npx tsc -p packages/loader/tsconfig.json
npx tsc -p packages/mdx/tsconfig.json
npx tsc -p packages/tag/tsconfig.json

[Himenon]

@silvenon

I see. understood. I wait until the PR is merged. Thank you.

[silvenon]

Merged! Feel free to continue with this.

[Himenon]

this issue resolved in https://github.com/mdx-js/mdx/pull/338