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

resolveJsonModule doesn't work #1109

Closed
pauldraper opened this issue Sep 7, 2019 · 18 comments
Closed

resolveJsonModule doesn't work #1109

pauldraper opened this issue Sep 7, 2019 · 18 comments
Labels
bug Can Close? We will close this in 30 days if there is no further activity

Comments

@pauldraper
Copy link

🐞 bug report

Affected Rule

ts_library

Is this a regression?

No.

Description

resolveJsonModule is not supported.

ts_library supports only .ts and .tsx sources.

https://www.typescriptlang.org/docs/handbook/compiler-options.html

@pauldraper pauldraper changed the title resolveJsonModule resolveJsonModule doesn't work Sep 7, 2019
@thesayyn
Copy link
Collaborator

thesayyn commented Sep 8, 2019

Matter of fact it does but it requires some configuration in tsconfig.json file.

Like so

{
  ...
  "bazelOptions": {
    "devmodeTargetOverride": "es2018",
    "googmodule": true
  }
}

Keep in mind this is just a workaround. worked well in my case.

@alexeagle
Copy link
Collaborator

I'm surprised that workaround works because I thought ts_library restricts inputs
https://github.com/bazelbuild/rules_nodejs/blob/master/packages/typescript/src/internal/build_defs.bzl#L279

@pauldraper
Copy link
Author

Yeah, it doesn't work (because of the reason both @alexeagle and I mentioned).

@Toxicable
Copy link

I think it hsould be reasonable to allow .json imports of tsc allows it

@marcus-sa
Copy link

Shouldn't .js be allowed as well?

@jwnewman12
Copy link

jwnewman12 commented Apr 17, 2020

Wouldn't this be blocked anyway due to the use of the UMD module type? Typescript doesn't allow resolveJsonModule with UMD. https://github.com/microsoft/TypeScript/pull/26825/files#diff-942ae3cd2a8bbd85ed86a60cd7c43307R7015 (I can't imagine why .. we are talking about static json files here, it shouldn't be this hard right).

error TS5071: Option '--resolveJsonModule' can only be specified when module code generation is 'commonjs', 'amd', 'es2015' or 'esNext'.

Our tsconfig.json has "module": "commonjs". I didn't understand the error the other day (we are new and trying to pilot bazel). I hastily removed resolveJsonModule and forgot about it.

Then, I set about patching this in to your rules (I couldn't add .json into src). Eventually, I think I got that right, the symlinks are finally there and valid. But still the json files weren't resolved during compilation, despite now being in place.

[I could publish this patch if interested, I believe it's close though I didn't update tests. It was at least useful for me to gain more of an understanding of how bazel works .. inputs and outputs].

Now I see I forgot about the setting in the tsconfig. I put that back, and now despite my efforts to patch I am still blocked, due to the above with UMD. I suppose I will try creating a json.d.ts file as described here https://stackoverflow.com/a/40473566 and try to move on.

@jwnewman12
Copy link

jwnewman12 commented Apr 20, 2020

I was able to get around it by creating a typings.d.ts file for as described in the link above. It's working at least. I had to update some of our imports.

-import * as data from 'data/obj.json';
+import * as data from '@lib/this-project-name/data/obj.json';

-import * as fixture from 'tests/fixtures/test-data.json';
+import * as fixture from '@test/this-project-name/tests/fixtures/test-data.json';

It's possible I may have had to do that anyway and it's unrelated to resolveJsonModule. Those paths were a bit un-anchored before and the @module thing is nice.

I now see that the 'devmode_module' option is there, I didn't explore setting this to non umd to see if resolveJsonModule + my patch actually worked.

But, for sure it would be nice if *.json and *.js were allowed in as pass through source files to ts_library. It would simplify some of our globs and would allow us to delete a few macros that the JS projects need; they could then just use the same ones for the TS projects.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs in two weeks. Collaborators can add a "cleanup" or "need: discussion" label to keep it open indefinitely. Thanks for your contributions to rules_nodejs!

@github-actions github-actions bot added the Can Close? We will close this in 30 days if there is no further activity label Oct 15, 2020
@longlho
Copy link
Contributor

longlho commented Oct 15, 2020

I believe this is fixed

@alexeagle
Copy link
Collaborator

yeah your #1995 I think, thanks

@vpanta
Copy link
Contributor

vpanta commented Oct 23, 2020

This is not fixed, as I'm running into it now on 2.0.1 (which should be after #1995, which only fixed this for ts_project I believe)

Is fixing this for ts_library a lost cause?

@alexeagle
Copy link
Collaborator

Yeah sorry still true for ts_library.
Can be fixed locally on the 3.x branch which no longer syncs with google3

@vpanta
Copy link
Contributor

vpanta commented Nov 2, 2020

I took a shot at it for ts_library, without necessarily needing to touch the code coming out of rules_typescript

Let me know what y'all think, we'll probably use a local branch of this for the time-being.

@tomasdev
Copy link

If I'm on 3.0.0-rc.1 what's the right way to use import data from 'data.json' ?

I think I'm able to do it via Rollup, but would like to have the concatjs_devserver also working.

@alexeagle
Copy link
Collaborator

@tomasdev which rule are you using that doesn't understand the json import?

@tomasdev
Copy link

tomasdev commented Dec 23, 2020

@alexeagle hey! thanks, I'm tracking it under #2365 -- it has an isolated test scenario as well (this ticket is about ts_library which doesnt support JSON and mine is about ts_project which supposedly does)

@github-actions github-actions bot removed the Can Close? We will close this in 30 days if there is no further activity label Jan 23, 2021
@github-actions
Copy link

This issue has been automatically marked as stale because it has not had any activity for 90 days. It will be closed if no further activity occurs in two weeks. Collaborators can add a "cleanup" or "need: discussion" label to keep it open indefinitely. Thanks for your contributions to rules_nodejs!

@github-actions github-actions bot added the Can Close? We will close this in 30 days if there is no further activity label Apr 27, 2021
@github-actions
Copy link

This issue was automatically closed because it went two weeks without a reply since it was labeled "Can Close?"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Can Close? We will close this in 30 days if there is no further activity
Projects
None yet
Development

No branches or pull requests

9 participants