Viijay-Kr / react-ts-css

VS Code Extension for CSS modules in typescript based react projects
https://marketplace.visualstudio.com/items?itemName=viijay-kr.react-ts-css
MIT License
43 stars 4 forks source link

Remove node_modules from published extension? #91

Open strlns opened 1 year ago

strlns commented 1 year ago

Hi, I'm not sure if I am completely misunderstanding something here, but it seems like you are including your node_modules folder in the published extension, which - AFAIK - should never be necessary.

https://code.visualstudio.com/api/working-with-extensions/bundling-extension#run-webpack

Related: https://github.com/Viijay-Kr/typescript-cleanup-defs/pull/1#issuecomment-1412634065 https://github.com/Viijay-Kr/react-ts-css/pull/90#issuecomment-1451890299

I'm not sure if or why this is necessary?

I'll look if I can provide a PR for this later.

Originally posted by @strlns in https://github.com/Viijay-Kr/react-ts-css/issues/89#issuecomment-1455172585

Viijay-Kr commented 1 year ago

@strlns Great Observation , the node modules folder wasn't shipped until we introduced a typescript server plugin to resolve #68 .

The plugin is enabled by the extension and is shipped with the node_modules hence it shouldn't be ignored by .vscodeignore.

The plugin filters our definition results from the declaration file provided by the starter kits like vite ,next, cra etc

Checkout the source code of Styled Components extension.

https://github.com/styled-components/vscode-styled-components/blob/main/.vscodeignore

Also there is a specific commit by one of the contributors to not ignore node_modules.

One alternative I see is to encourage our users to use the plugin directly inside their tsconfig.json.

If there is another alternative to this then I'd be happy to have a PR

strlns commented 1 year ago

Hi @Viijay-Kr, thank you for your response and indeed I also found node_modules in the published package of the styled components extension.

I'm not sure if I already understand how VSCode extensions hook into the TS language server without using a workspace TS version.

I'll check out some more documentation on this as I don't want to jump to invalid conclusions.

Casually observed, this package here still weighs a lot more than the packaged extension at https://marketplace.visualstudio.com/items?itemName=styled-components.vscode-styled-components. This just makes me wonder if build dependencies are shipped.

The deprecation warning on that other package page confuses me quite a bit, as the link is circular.

I'll leave this open until I understand the problem better, thank you for the hints.

karlhorky commented 1 year ago

In case it fits the project goals of the React CSS modules extension, there would be ways to avoid including the node_modules, such as bundling the code before publishing (eg. using esbuild or something like ncc)

strlns commented 1 year ago

@karlhorky if I understand @Viijay-Kr correctly, the added TS language server plugin is the reason why node_modules is included, see https://github.com/Viijay-Kr/react-ts-css/blob/c31f0616cde83bc6b4692c9de1b809729310e22c/package.json#L154 (where the plugin is registered).

The code is already bundled via Webpack and the main file loaded by VSCode is the compiled file dist/extension.js. https://github.com/Viijay-Kr/react-ts-css/blob/c31f0616cde83bc6b4692c9de1b809729310e22c/package.json#L62

We'd need a way to reference bundled code in the line of my first link instead of using Node module resolution.

Or prune the bundled node_modules to only include the relevant module.

Currently it seems like everything from dependencies (not devDependencies) is included.

Not sure if that was helpful, currently I'm not sure if the other entries there are needed at runtime in VSCode context or if they are already bundled by webpack into the dist bundle.

Viijay-Kr commented 1 year ago

@strlns Only the TS plugin is needed inside the node_module. The rest is not needed. So moving the other dependencies to dev depending should minimize the load on node_modules

strlns commented 1 year ago

Bit of a tradeoff here 🤔 Since I'm usually a fan of listing stuff that ends up in the bundle in dependencies, I don't want to change your dependency categorization. If I understand correctly, this should be a job for .vscodeignore?

Sorry that I'm spending more time writing messages instead of submitting PRs. Currently have to stay in bed because of an injury and haven't got too much energy.

Maybe just add node_modules there and then unignore the TS plugin?

Viijay-Kr commented 1 year ago

If I understand correctly, this should be a job for .vscodeignore?

yes.

VS code expects us to ignore node_modules by default. So it doesn't matter if the other dependencies are moved to dev dependencies since they are dev mode only .

The extension is shipped as a production bundle using webpack with this one little caveat of including the plugin.

So @strlns I wouldn't stress to much on this one as the extension worked well before the introduction of the node_modules which was done after #70.