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

TS files outside project dir not being parsed -- microbundle uses outdated version of rpt2 #295

Closed
asas-virtuais opened this issue Dec 27, 2021 · 6 comments
Labels
kind: support Asking for support with something or a specific use case problem: removed issue template OP removed the issue template without good cause scope: downstream Issue in a downstream library, not in this library itself solution: duplicate This issue or pull request already exists solution: outdated This is not up-to-date with the current version topic: monorepo / symlinks Related to monorepos and/or symlinks (Lerna, Yarn, PNPM, Rush, etc)

Comments

@asas-virtuais
Copy link

What happens and why it is wrong

Environment

TS files outside the project directory being parsed as Flow files instead of TS

SyntaxError: /absolute/path/to/file.ts: Support for the experimental syntax 'flow' isn't currently enabled (2:8):

I am experiencing this when using microbundle to bundle my monorepo projects/packages

Original issue:
developit/microbundle#808

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

@asas-virtuais asas-virtuais changed the title TS files outside the project directory being parsed as Flow files instead of TS [Repro available] TS files outside the project directory being parsed as Flow files instead of TS Dec 27, 2021
@agilgur5 agilgur5 changed the title [Repro available] TS files outside the project directory being parsed as Flow files instead of TS TS files outside project dir not being parsed Apr 23, 2022
@agilgur5
Copy link
Collaborator

agilgur5 commented Apr 23, 2022

So the syntax error you've written down here isn't from rpt2 but from Babel -- that's not an rpt2 error.

I'm reading the downstream issue though and I've rewritten the title to match. Per downstream, it sounds like this may be a duplicate of #216

You're also using paths which complicate things a little bit too (see #201 ), on top of the repro being a microbundle repro and not a minimal rpt2 repro

@agilgur5 agilgur5 added the topic: monorepo / symlinks Related to monorepos and/or symlinks (Lerna, Yarn, PNPM, Rush, etc) label May 1, 2022
@agilgur5 agilgur5 added the problem: removed issue template OP removed the issue template without good cause label May 27, 2022
@agilgur5
Copy link
Collaborator

agilgur5 commented May 27, 2022

I recently fixed a big issue with symlinks in #332, so I decided to investigate more of the monorepo / symlink issues to see if they were fixed as well or if I could fix them.

I created a fork here with some small modifications (I don't believe they actually affect anything, only the include change is necessary to get tsc to work) to help me debug and get to root cause by removing & consolidating config.

Added logging

I then modified the local rpt2 installation within node_modules to test a bunch of things.
In particular I upped the verbosity to 3 -- the issue template requests this debug log -- to get better logging (also set clean: true in case of cache issues), and saw this right before the error:

rpt2: resolving 'bundle-bug-repro/modules/code' imported by '<path omitted>/bundle-bug-repro/packages/some-package/index.ts'
rpt2:     to '<path omitted>/bundle-bug-repro/modules/code.ts'

No logging for transform hook

Basically, it was able to resolve this file, and did so properly too (great!), but, notably it didn't transform the file as no logging was output for the transform hook.

rpt2 filters it out

I suspected that this might be due to the transform hook's filter filtering out the file, as it's designed not to touch this unrelated to this plugin.
I added some some logging in that conditional, and indeed it gets filtered out:

rpt2: resolving 'bundle-bug-repro/modules/code' imported by '<path omitted>/bundle-bug-repro/packages/some-package/index.ts'
rpt2:     to '<path omitted>/bundle-bug-repro/modules/code.ts'
rpt2: filtering out transform for '<path omitted>/bundle-bug-repro/modules/code.ts'

Duplicate of #216

The Babel error occurs after that because microbundle adds @rollup/plugin-babel as a plugin after rpt2 (I solo-maintained TSDX for ~1.5 years so I'm quite familiar with those internals).
When a plugin returns on a hook, Rollup just passes it on to the next plugin: in this case, plugin-babel's transform hook. It does not filter it out and then it fails to parse as preset-typescript isn't used (since rpt2 is supposed to go first), resulting in this error.

In a more minimal reproduction such as #216, you'll get a Rollup error here instead:

Error: Unexpected token (Note that you need plugins to import files that are not JavaScript)

That's because Rollup can't parse it itself and rpt2 didn't transform it to ESM JS as it filtered it out

So I can definitively confirm that this duplicates #216 now.

Re: #332

#332 is unrelated to this as that fixes the transform hook's resolution of symlinks, and this doesn't even reach that part as it's filtered out beforehand.
It's good to see here that the resolveId hook doesn't run into problems with symlinks, meaning that the way we're using TS's moduleNameResolver handles symlinks. This answers a question posed in #234 (that was ultimately a red herring).

That being said, if this issue were fixed, you might very well run into the issue that #332 fixes (i.e. #234) right after due to the usage of pnpm symlinks. So it's likely related to your issue, but not the current root cause.

Tried to get it to work

tsc works... might actually be a microbundle config issue for monorepos like this as such

tsc works for this file and outputs the correct code and typings. rpt2 should try to reflect tsc as much as possible, though the filter is an intentional Rollup semantic. It may be incorrectly applied that being said.

But if you were using this plugin directly, you could configure the include and exclude (which is what filter is based on) of the plugin yourself. So the fix for this may actually be in microbundle's configuration of rpt2, in that it may need to handle monorepos like this by configuring rpt2's include/exclude.

attempted to workaround filter

So to workaround the filter, I tried a few things unsuccessfully:

  1. Setting rootDir to the root of the repo, in order to use fix: use compilerOptions.rootDir to filter files #249
  2. Using TS project references to utilize Add support for project references #139 (comment)
    1. Setting baseUrl on top of this per cannot build package with project references #260 (comment). To be explicit, this actually didn't change anything; I suspect that issue might be user misconfiguration due to how extends works confusingly with relative paths (c.f. The compilerOptions.outDir config is incorrectly resolved when in a shareable config microsoft/TypeScript#29172).

I'm not 100% why these still got filtered out, but per #329 / #321 (comment), Rollup's createFilter is actually pretty confusing.

Here's the logs from using references:

rpt2: included:
[
    "*.ts+(|x)",
    "**/*.ts+(|x)",
    "<path omitted>/bundle-bug-repro/*.ts+(|x)",
    "<path omitted>/bundle-bug-repro/**/*.ts+(|x)"
]
rpt2: excluded:
[
    "*.d.ts",
    "**/*.d.ts",
    "<path omitted>/bundle-bug-repro/*.d.ts",
    "<path omitted>/bundle-bug-repro/**/*.d.ts"
]
rpt2: resolving 'bundle-bug-repro/modules/code' imported by '<path omitted>/bundle-bug-repro/packages/some-package/index.ts'
rpt2:     to '<path omitted>/bundle-bug-repro/modules/code.ts'
rpt2: filtering out transform for '<path omitted>/bundle-bug-repro/modules/code.ts'

So it's in the include as far as I can tell, but still filtered out...

I'll continue to investigate the above, but still closing this as duplicate of #216. If I can figure out how to get filter working, it would apply to this issue and #216. But the result may require a downstream fix from microbundle. I'm not totally sure yet since I haven't quite figured it out yet.

@agilgur5 agilgur5 added the solution: duplicate This issue or pull request already exists label May 27, 2022
@agilgur5
Copy link
Collaborator

Got it to work

Can see my fork and read through the detailed commit log to understand it, but I'll summarize here too.

microbundle uses an outdated version of rpt2

microbundle is using an older version of rpt2 from ~1.5 years ago, 0.29.0.
Notably, 0.30.0 updated the @rollup/pluginutils dep to v4, which contains a breaking change in rollup/plugins#517. With that change to filter, it now resolves files outside of the cwd.

I'll report downstream to the microbundle folks that they should probably update to fix some issues like these.

empty chunk

With the pnpm override added, rpt2 resolves the file correctly and there is no error anymore... but it produces an empty chunk! But rpt2 does actually transform the code properly; here's the output of some logs I added on this line 🤔 :

rpt2: transformResult: '{"code":"export * from 'bundle-bug-repro/modules/code';\r\n","map":{"mappings":""}}'

That was pretty perplexing for a while, but it turns out that's actually correct behavior though because Rollup applies treeshaking and the export does nothing. This is because export * from ... actually doesn't include the default export, and hence it is empty and treeshaken.
I changed this to export { default } from ... and then it outputs a correct non-empty file 😉

workaround in older rpt2 exists too, setting rootDir

The older rpt2 that microbundle uses has a workaround for this too, using rootDir as was implemented in #249. I confirmed that this works in your repo when you add "rootDir": "../../" to some-package's tsconfig.json.

Notably, in my last comment I couldn't get this to work for some reason... but that's actually because I made a coding error while modifying the local rpt2 😅 (I didn't add braces to a previously one-line conditional when I added logging to it and completely broke the transform hook as such).
Project references don't quite work in this version, but I think that's because rollup/plugins#517 fixes absolute paths as well, so that's probably due to the older version too.

@agilgur5 agilgur5 added solution: outdated This is not up-to-date with the current version scope: downstream Issue in a downstream library, not in this library itself kind: support Asking for support with something or a specific use case labels May 27, 2022
@agilgur5 agilgur5 changed the title TS files outside project dir not being parsed TS files outside project dir not being parsed -- microbundle uses outdated version of rpt2 May 27, 2022
@agilgur5
Copy link
Collaborator

Also can confirm that #216 works with a newer rpt2 version as well, so both of these are resolved 🙂

@agilgur5
Copy link
Collaborator

agilgur5 commented Jun 2, 2022

Submitted a PR downstream to fix this in microbundle: developit/microbundle#967

@asas-virtuais
Copy link
Author

Thank you so much @agilgur5, you inspired me today! Hope to collaborate again in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: support Asking for support with something or a specific use case problem: removed issue template OP removed the issue template without good cause scope: downstream Issue in a downstream library, not in this library itself solution: duplicate This issue or pull request already exists solution: outdated This is not up-to-date with the current version topic: monorepo / symlinks Related to monorepos and/or symlinks (Lerna, Yarn, PNPM, Rush, etc)
Projects
None yet
Development

No branches or pull requests

2 participants