developit / microbundle

📦 Zero-configuration bundler for tiny modules.
https://npm.im/microbundle
MIT License
8.06k stars 361 forks source link

Breaks when trying to import typescript outside of directory #808

Closed n8jadams closed 2 years ago

n8jadams commented 3 years ago

I'm working in a typescript monorepo structured sort of like this

.
├── backend
├── frontend
├── shared
└── package
   ├── package.json
   └── tsconfig.json

I'm getting weird errors whenever in my package code I try to import a typescript module from outside the package directory. It's like microbundle doesn't know that it's a typescript file.

For example, I import some typescript types that are generated by graphql-code-generator and live in the top level shared directory. When I import the file and run my build script I get this error message:

(babel plugin) SyntaxError: /absolute/path/to/generated-types.ts: Support for the experimental syntax 'flow' isn't currently enabled (2:8):

  1 | import { GraphQLResolveInfo, GraphQLScalarType, GraphQLScalarTypeConfig } from 'graphql';
> 2 | export type Maybe<T> = T | null;
    |        ^
  3 | export type Exact<T extends { [key: string]: unknown }> = { [K in keyof T]: T[K] };
  4 | export type MakeOptional<T, K extends keyof T> = Omit<T, K> & { [SubKey in K]?: Maybe<T[SubKey]> };
  5 | export type MakeMaybe<T, K extends keyof T> = Omit<T, K> & { [SubKey in K]: Maybe<T[SubKey]> };

Add @babel/preset-flow (https://git.io/JfeDn) to the 'presets' section of your Babel config to enable transformation.
If you want to leave it as-is, add @babel/plugin-syntax-flow (https://git.io/vb4yb) to the 'plugins' section to enable parsing.

at undefined:2:7

It looks like microbundle doesn't know this is typescript and thinks it's flow...

However, when I copy the contents of that file into the package directory and import that instead, it builds just fine. Same with a symlink to that file from within the package.

Just to be certain, I explicitly added the relative path to the include array in the packages/tsconfig.json, and got the same error.

Why is microbundle unable to understand imports of files outside of the package directory? Or am I missing some configuration or something like that?

ashkanhosseini commented 3 years ago

I wonder if you could find a remedy to this? I have the same issue. I'm trying to include some external packages in the bundle because I'm in a monorepo and get the exact same error.

asasvirtuais commented 2 years ago

I am also experiencing this

rschristian commented 2 years ago

Just to check as no one's provided a reproduction, are people seeing this when importing types/interfaces or only when importing things that would be transpiled to JS?

I cannot reproduce the former, but can the latter.

Edit: Seems like it might be an upstream issue with rpt2, see https://github.com/ezolenko/rollup-plugin-typescript2/issues/216. It seems like exporting types/interfaces from other directories is fine, but not anything else.

n8jadams commented 2 years ago

My experience has been anything that is Typescript breaks it (including types/interfaces) while regular JS works fine.

asasvirtuais commented 2 years ago

My experience has been anything that is Typescript breaks it (including types/interfaces) while regular JS works fine.

That relates to what I am doing. When I tried importing typescript files from outside the parent directory I got that error.

I had to move the files inside the project root directory to import them successfully.

I will try to provide a repro as soon as possible

rschristian commented 2 years ago

Interesting, might be an issue if the TS file imports items itself? Can definitely import pure types/interfaces from an external (to the project) module without issue, though, worth noting that I don't believe these types end up being built into your build output, so more of a "doesn't impede the build" sort of thing than "works".

Regardless a repro would help at least to help narrow down these issues/behaviors. I don't think there's much we can do, but knowing the limits would be better.

asasvirtuais commented 2 years ago

Here I am again facing this exact problem, sorry too busy to make that repro

asasvirtuais commented 2 years ago

@rschristian

I will give more context

I hate having to transpile my typescript files for every library I make up, I would have to maintain a dozen of libraries and transpile them to CJS and ESM and that gets annoying. So I just said fuck it and instead of writing libraries I write typescript code under workspace/modules. Bear with me, I will explain what I mean.

I work with the following structure

workspace/
    modules/
        my-pseudo-library/
            awesome-code.ts
    projects/
        my-cool-project/
                src/index.ts                   // This imports from workspace/modules/my-pseudo-library/awesome-code.ts
                package.json

Well then, inside my-cool-project I use microbundle to bundle all that code into a single bundle.js file

Unfortunately, I am running into this issue where what seems to be a bug is causing microbundle to treat my typescript code that is external to the project as Flow code instead of typescript code, throwing syntax errors like the one OP mentioned

rschristian commented 2 years ago

Well, as I've said, you should be able to import external types (at least I can in all of my tests), but this issue is upstream. Not much we can do about it.

asasvirtuais commented 2 years ago

Good morning (for me), I've made a repro for this.

https://github.com/icaro-capobianco/bundle-bug-repro

I will also be investigating the cause

asasvirtuais commented 2 years ago

@rschristian Question: does microbundle tell @babel if the code being parsed is typescript or flow? Or is that something @babel figures out on its own?

rschristian commented 2 years ago

That's something that Babel handles. Parsing is its job.

Regardless, TS is handled by rpt2. We don't use Babel to transpile AFAIK.

agilgur5 commented 2 years ago

So @icaro-capobianco created https://github.com/ezolenko/rollup-plugin-typescript2/issues/295 referencing this downstream issue and I recently investigated and fixed Ícaro's repro in a fork. I help maintain rpt2 nowadays and formerly solo-maintained TSDX (which is how I originally came to contribute to rpt2 over the years), so I'm very familiar with microbundle's internals as well, given that TSDX is effectively a fork.

I did a root cause analysis in https://github.com/ezolenko/rollup-plugin-typescript2/issues/295#issuecomment-1139994454 and it turns out this was due to microbundle using an outdated version of rpt2. Per my RCA there, upgrading rpt2 fixes this issue, and setting tsconfig rootDir is a workaround in older versions of rpt2.

I submitted a PR in #967 to update rpt2 to the latest release v0.32.0, which fixes two different issues with monorepos (this and symlinks).

We don't use Babel to transpile AFAIK.

Not to transpile TS, but it runs after rpt2 in the plugin chain in order to support Babel plugins and preset-env (a simple graph: ts -> rpt2 -> esnext -> babel/preset-env -> js transpiled for targets). That is why this issue gives a Babel error. With older versions of rpt2, it filters out files outside of the project directory, then the next plugin in the chain (Babel) runs, and since it's not set-up to parse TS but was given a TS file, it throws this error. You won't get this error running just rpt2, such as in the minimal repro, as Babel isn't ran. The bit about Flow is confusing, but I believe that's because preset-react still includes Flow plugins (that are by default disabled).

rschristian commented 2 years ago

Not to transpile TS, but it runs after rpt2 in the plugin chain in order to support Babel plugins and preset-env (a simple graph: ts -> rpt2 -> esnext -> babel/preset-env -> js transpiled for targets).

Maybe mixing up my issues here, but I thought this failed even with types-only files, that would never be part of the graph as they contain no JS. Fair enough though.

The bit about Flow is confusing, but I believe that's because preset-react still includes Flow plugins (that are by default disabled).

Ah, interesting. Good to know.

agilgur5 commented 2 years ago

Maybe mixing up my issues here, but I thought this failed even with types-only files, that would never be part of the graph as they contain no JS. Fair enough though.

You'd be referring to the long-standing https://github.com/ezolenko/rollup-plugin-typescript2/issues/7 and its various related issues, such as https://github.com/ezolenko/rollup-plugin-typescript2/issues/211 and https://github.com/ezolenko/rollup-plugin-typescript2/issues/298 where I began a root cause analysis and actually have more or less figured out how to solve this (will do a RCA write-up and maybe draft PR soon, but it requires a heck of a lot of testing, so think I'll need to build out an integration test suite prior to release).

But that has nothing to do with microbundle's plugin chain, so I think you are mixing up issues, but I'm also not really sure what the direction of that comment was either? Fwiw, monorepos and type-only imports are basically rpt2's two most common issues, so maybe you were just thinking of the other one. I'm sure there's an issue (or issues) in microbundle that track the upstream type-only issue in rpt2 as well, just not this one.

The bit about Flow is confusing, but I believe that's because preset-react still includes Flow plugins (that are by default disabled).

Actually I was wrong on this one, the stacktrace shows it comes from babel-parser actually, which has the capability to recognize and throw an error on unsupported syntaxes. Code with type could be either TS or Flow, so I suppose the error message just chose one (Babel also used to be written in Flow before migrating to TS and some parts have not yet been migrated. I migrated babel-polyfills to TS two months or so ago). I think newer versions of Babel throw different error messages though.

rschristian commented 2 years ago

You'd be referring to the long-standing https://github.com/ezolenko/rollup-plugin-typescript2/issues/7 and its various duplicates

It was this issue, just my memory of the issue was backwards it looks like: https://github.com/developit/microbundle/issues/808#issuecomment-972589329

Type-only imports were a-ok.

agilgur5 commented 2 years ago

It was this issue, just my memory of the issue was backwards it looks like: #808 (comment)

Type-only imports were a-ok

Ah I see. In the specific case of this issue, type-only imports probably didn't cause problems because they don't currently run through the transform hook at all (due to https://github.com/ezolenko/rollup-plugin-typescript2/issues/211). That hook is where the filter that caused this issue is located.

Basically, importing a type-only file outside of the project dir wouldn't encounter this issue bc rpt2 doesn't transform/report diagnostics for such files anyway due to https://github.com/ezolenko/rollup-plugin-typescript2/issues/211.

In other words, one issue masked another issue 😅

Brief summary of my WIP RCA for the type-only issue for those interested
I believe it occurs because the _compiled_ JS doesn't contain imports to the type-only files. So, when Rollup parses that compiled code, it never calls the `resolveId` hook for those imports (as they don't exist in Rollup's ESM-only eyes) and so they're not resolved or transformed etc. While ezolenko did implement a partial workaround to use the list of files from `tsconfig` `include` years ago, it doesn't fully handle this (type-checking is missed I believe, but declarations are produced), doesn't necessarily capture all type-only files, and doesn't work with the `tsconfig` `files` allowlist instead, etc. So the solution is almost certainly going to be to parse the import chain within rpt2 itself with `ts.preProcessFile`, since, in this case, Rollup can't handle that as it normally does. Can stay tuned into that issue for more details over time. **EDIT**: see full RCA in https://github.com/ezolenko/rollup-plugin-typescript2/issues/298#issuecomment-1146658442