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

Dynamic import issue for TypeScript 4.5.0-beta "module" of "nodenext" or "node12" #16

Closed
4 tasks done
tvquizphd opened this issue Nov 2, 2021 · 8 comments
Closed
4 tasks done
Labels
🙅 no/wontfix This is not (enough of) an issue for this project 👎 phase/no Post cannot or will not be acted on

Comments

@tvquizphd
Copy link

Initial checklist

Affected packages and versions

hastscript==7.0.2

Link to runnable example

https://github.com/tvquizphd/hastscript-test

Steps to reproduce

In index.ts, I simply import h from hastscript:

import { h } from 'hastscript' 

Here's a relevant tsconfig.json snippet:

  "compilerOptions": {
    "target": "es6",
    "outDir": "dist",
    "declaration": true,
    "module": "node12"
  }

Here's a relevant package.json snippet:

 "dependencies": {
   "hastscript": "^7.0.2"
 },
 "devDependencies": {
   "typescript": "^4.5.0-beta"
 }

Expected behavior

The module should build without error.

Actual behavior

There seems to be an error with dynamic import of jsx-classic in html.d.ts and svg.d.ts:

yarn run v1.22.17
warning package.json: No license field
$ tsc
node_modules/hastscript/lib/html.d.ts(19,27): error TS2307: Cannot find module './jsx-classic' or its corresponding type declarations.
node_modules/hastscript/lib/html.d.ts(20,39): error TS2307: Cannot find module './jsx-classic' or its corresponding type declarations.
node_modules/hastscript/lib/html.d.ts(21,37): error TS2307: Cannot find module './jsx-classic' or its corresponding type declarations.
node_modules/hastscript/lib/html.d.ts(23,14): error TS2307: Cannot find module './jsx-classic' or its corresponding type declarations.
node_modules/hastscript/lib/svg.d.ts(19,27): error TS2307: Cannot find module './jsx-classic' or its corresponding type declarations.
node_modules/hastscript/lib/svg.d.ts(20,39): error TS2307: Cannot find module './jsx-classic' or its corresponding type declarations.
node_modules/hastscript/lib/svg.d.ts(21,37): error TS2307: Cannot find module './jsx-classic' or its corresponding type declarations.
node_modules/hastscript/lib/svg.d.ts(23,14): error TS2307: Cannot find module './jsx-classic' or its corresponding type declarations.
error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this comman

Here are lines 17-25 of node_modules/hastscript/lib/html.d.ts:

export namespace h {
  namespace JSX {
    type Element = import('./jsx-classic').Element
    type IntrinsicAttributes = import('./jsx-classic').IntrinsicAttributes
    type IntrinsicElements = import('./jsx-classic').IntrinsicElements
    type ElementChildrenAttribute =
      import('./jsx-classic').ElementChildrenAttribute
  }
}

Runtime

Node v16

Package manager

Other (please specify in steps to reproduce)

OS

Linux

Build and bundle tools

Other (please specify in steps to reproduce)

@github-actions github-actions bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Nov 2, 2021
@tvquizphd tvquizphd changed the title Dynamic import issue in lib/html.d.ts and lib/svg.d.ts Dynamic import issue for TypeScript "module" of "node" or "node12" Nov 2, 2021
@tvquizphd
Copy link
Author

tvquizphd commented Nov 2, 2021

I should note that this issue is only for Typescript ^4.5.0-beta "module" of "nodenext" or "node12".

The recommended option of "module": "commonjs" builds correctly.

@tvquizphd tvquizphd changed the title Dynamic import issue for TypeScript "module" of "node" or "node12" Dynamic import issue for TypeScript "module" of "nodenext" or "node12" Nov 2, 2021
@tvquizphd tvquizphd changed the title Dynamic import issue for TypeScript "module" of "nodenext" or "node12" Dynamic import issue for TypeScript 4.5.0-beta "module" of "nodenext" or "node12" Nov 2, 2021
@ChristianMurphy
Copy link
Member

Thanks for reaching out @tvquizphd!
And for testing things out with the new TS beta.

A few things to note here:

  1. 4.5.0-beta, is exactly that a beta, this in particular looks like it may be a bug in TS itself (my understanding is the extension requirement applies to implementation files, not d.ts files)
  2. The issue may have already been fixed, when I install 4.5.0-beta switch module to "module": "nodenext", and run the type tests in this repo no error is thrown
    "test": "npm run build && npm run generate && npm run format && npm run test-coverage"
  3. The status of node12 and nodenext options is a bit tenuous at the moment Concerns with TypeScript 4.5's Node 12+ ESM Support microsoft/TypeScript#46452 (hopefully the issues can be resolved), I'm cautious about making changes for this mode specifically until it stabilizes.
  4. "module": "commonjs" is not recommended, "module": "es2020" is https://gist.github.com/sindresorhus/a39789f98801d908bbc7ff3ecc99d99c#how-can-i-make-my-typescript-project-output-esm

I'd be fine updating

* @typedef {import('./core.js').HProperties} Properties Acceptable properties value.
*
* @typedef {import('./jsx-classic').Element} h.JSX.Element

with

-   * @typedef {import('./jsx-classic').Element} h.JSX.Element 
+   * @typedef {import('./jsx-classic.js').Element} h.JSX.Element

including the extension for consistency with other files.
But I am cautious making any other changes at this time.

@ChristianMurphy ChristianMurphy added the ☂️ area/types This affects typings label Nov 2, 2021
@tvquizphd
Copy link
Author

@ChristianMurphy --- Thanks for the clarity!

I was able to see my mistake. I need to set moduleResolution to "node" in tsconfig.json. This allows the import of hastscript with no issues for "module": "node12".

The error message when "module": "ES2020" is was helpful:

Cannot find module 'hastscript'. Did you mean to set the 'moduleResolution' option to 'node', or to add aliases to the 'paths' option?

Also, the error message when "moduleResolution": "node12" is even more helpful:

Relative import paths need explicit file extensions in EcmaScript imports when '--moduleResolution' is 'node12' or 'nodenext'

With the Typescript beta, to support the "node12" in both module and moduleResolution, I guess hastscript would need to add explicit file extensions to all relative imports. For now, users of hastscript can use "module": "node12" and "moduleResolution": "node".

@tvquizphd
Copy link
Author

From the Tyepscript 4.5.0-beta release notes, it seems they explicitly say that we should import *.ts files and *.d.ts files from the filename with the extension switched to .js.

// ./bar.ts
import { helper } from "./foo.js"; // works in ESM & CJS

...
One other thing to mention is the fact that this applies to .d.ts files too. When TypeScript finds a .d.ts file in package, it is interpreted based on the containing package.

@wooorm
Copy link
Member

wooorm commented Dec 30, 2021

Didn’t they revert that change in 4.5.0? If it lands, and it can be fixed here, 👍 to a PR

@wooorm
Copy link
Member

wooorm commented Apr 1, 2022

I’ll close this for now, as it seems reverted. PR welcome if they land this again.

@wooorm wooorm closed this as completed Apr 1, 2022
@wooorm wooorm added 🙅 no/wontfix This is not (enough of) an issue for this project and removed ☂️ area/types This affects typings 🤞 phase/open Post is being triaged manually labels Apr 1, 2022
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions github-actions bot added the 👎 phase/no Post cannot or will not be acted on label Apr 1, 2022
helmturner added a commit to helmturner/remark-validate-vfile that referenced this issue Jul 8, 2022
helmturner added a commit to helmturner/remark-morematter that referenced this issue Jul 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🙅 no/wontfix This is not (enough of) an issue for this project 👎 phase/no Post cannot or will not be acted on
Development

No branches or pull requests

3 participants