You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
unable to import from installed library in node_modules without relative path #1954
git clone https://github.com/lume/asdom.git
cd asdom
git checkout unable-to-import
npm install
cd example
npm install
npm run dev // ERROR
The following works, but TypeScript shows red squigglies and intellisense breaks because AssemblyScript projects are not set up by default to satisfy TypeScript node resolution:
import{...}from'asdom'// red squigglies all over!
The following doesn't work in AS, but it is proper and TypeScript will see the correct imports and we will get working intellisense:
import{...}from'asdom/assembly/index'// Intellisense works, but not AS
If assembly is the base directory there, I think a working (for AS) import would be
import{...}from'asdom/index'
while asdom/assembly/index would attempt to find asdom/assembly/assembly/index.ts. Perhaps that's the culprit we have been looking at here for a while, in that leaving out the assembly would allow it to compile, but is not TS compatible?
Yep, that. Types not found in VS Code. Requires an exports field in package.json to also be compatible with Node-style imports, which TypeScript doesn't support yet.
Why not just?
import{}from"asdom"
A few reasons:
Some things are not exported from the index for various reasons.
That is still not compatible with TypeScript by default, breaks VS Code intellisense out of the box.
For TS/JS-portable code, importing from a package index/main file is bad practice: it imports everything by default and does not tree-shake like the AS compiler does out of the box. This means that by default (for example with native ES Modules) an end user will import everything from a library even if they use only 1% of the library. For example, the statement import {oneLittleThing} from 'some-huge-library' is not so good, especially with big libraries where you don't need everything, because the entire module graph will be imported unnecessarily (and with native ES modules this means more network requests).
You could keep ascMain for the cases where people don't want assembly/index.ts but regardless of having it as-package/subpackage should resolve to dirname(entry of as-package)/subpackage.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in one week if no further activity occurs. Thank you for your contributions!
It's not purely a bug, as it is kinda by design, but the design itself is kinda a bug, so I guess it is one. We'd need to revert the baseDir computation based on ascMain, which would be a breaking change.
I've spent some time looking at this problem now, and came to the conclusion that removing ascMain is most likely the most sensible approach. Regardless of whether some of its current problems can be fixed, it will very likely never integrate with existing TS tooling. Without such integration, there'll always be red squigglies all over, either because types cannot be obtained, or wrong types are obtained where TS would do something different than AS.
Hence going to remove the feature, with documentation soon recommending to simply do import from "myPackage/assembly" or any other inner path, as it is an approach that always works.
As a possible path forwards in the future, Node.js nowadays recognizes the exports property in package.json (main is considered legacy iiuc), looking like
Activity
trusktr commentedon Jul 6, 2021
Updated the reproduction to checkout a particular branch.
dcodeIO commentedon Jul 6, 2021
If
assemblyis the base directory there, I think a working (for AS) import would bewhile
asdom/assembly/indexwould attempt to findasdom/assembly/assembly/index.ts. Perhaps that's the culprit we have been looking at here for a while, in that leaving out theassemblywould allow it to compile, but is not TS compatible?yjhmelody commentedon Jul 6, 2021
@trusktr May be similar to this question MaxGraey/as-string-sink#1
willemneal commentedon Jul 6, 2021
Why not just?
trusktr commentedon Jul 8, 2021
Yep, that. Types not found in VS Code. Requires an
exportsfield in package.json to also be compatible with Node-style imports, which TypeScript doesn't support yet.A few reasons:
import {oneLittleThing} from 'some-huge-library'is not so good, especially with big libraries where you don't need everything, because the entire module graph will be imported unnecessarily (and with native ES modules this means more network requests).willemneal commentedon Jul 8, 2021
Does
types:assembly/index.tswork for 2?So really this issue here is that we can't do
asdom/submodule?Also it sounds like tree shaking is very bad for TS/JS.
JairusSW commentedon Jul 8, 2021
I have the same thing.
Should be:
The
../node_modulewon't work well for people using different package managers (pnpm for example). :'(dcodeIO commentedon Jul 9, 2021
Seems to go into the direction of finding a better convention, and deprecating
ascMain- no? :)willemneal commentedon Jul 9, 2021
You could keep
ascMainfor the cases where people don't wantassembly/index.tsbut regardless of having itas-package/subpackageshould resolve todirname(entry of as-package)/subpackage.github-actions commentedon Aug 8, 2021
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in one week if no further activity occurs. Thank you for your contributions!
trusktr commentedon Aug 11, 2021
Not stale. 🤖💥• • • • 💥🔫
dcodeIO commentedon Aug 11, 2021
It's not purely a bug, as it is kinda by design, but the design itself is kinda a bug, so I guess it is one. We'd need to revert the baseDir computation based on ascMain, which would be a breaking change.
dcodeIO commentedon Dec 9, 2021
I've spent some time looking at this problem now, and came to the conclusion that removing
ascMainis most likely the most sensible approach. Regardless of whether some of its current problems can be fixed, it will very likely never integrate with existing TS tooling. Without such integration, there'll always be red squigglies all over, either because types cannot be obtained, or wrong types are obtained where TS would do something different than AS.Hence going to remove the feature, with documentation soon recommending to simply do
import from "myPackage/assembly"or any other inner path, as it is an approach that always works.As a possible path forwards in the future, Node.js nowadays recognizes the
exportsproperty inpackage.json(mainis considered legacy iiuc), looking likewhere, perhaps, if typical tooling would support it, there could be an
"assembly"entry. Likely not going to happen anytime soon, though :)