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

Plugin template scaffold generates package.json for easier publish #3548

Closed
kwonoj opened this issue Feb 12, 2022 · 19 comments · Fixed by #3566
Closed

Plugin template scaffold generates package.json for easier publish #3548

kwonoj opened this issue Feb 12, 2022 · 19 comments · Fixed by #3566

Comments

@kwonoj
Copy link
Member

kwonoj commented Feb 12, 2022

Describe the feature

#3540 (comment)

When we create a plugin via template, we expect it'll be published as npm package in most cases. Having default package.json would be helpful.

We could do

  • create package.json by default
  • having a flag to indicate to create package.json.

For now I'm leaning on first approach, as said expect most of cases we expect plugin will be published as npm pkg and installed next to @swc/core.

Babel plugin or link to the feature description

No response

Additional context

No response

@kdy1
Copy link
Member

kdy1 commented Feb 13, 2022

How should we manage package.json?
e.g. For the package version, which one should we use?

I was thinking generating one on build command, just like wasm-bindgen does

@kwonoj
Copy link
Member Author

kwonoj commented Feb 13, 2022

I'd rather let it be created on scaffolding time, and plugin author to manage further details on their own. Same as cargo / our plugin scaffolding does for cargo.toml.

It may be neat to try cli becomes all handlers for the feature set like build, metadata mgmt / publishing but that's something we can consider in a long run, moreover I'm not sure if that's something we are willing to support as core cli feature. The assumption for the plugin is people are well aware of npm's ecosystem as natually this is being used for node.js binding of swc (@swc/core).

@kdy1
Copy link
Member

kdy1 commented Feb 13, 2022

I see, seems reasonable to me.

@kdy1
Copy link
Member

kdy1 commented Feb 13, 2022

I think a command to move/copy wasm files to the package directory would be enough. (Currently moving wasm using bash in monorepo is hard)

@kwonoj kwonoj added this to the v2.0.0-alpha.1 milestone Feb 13, 2022
@kwonoj
Copy link
Member Author

kwonoj commented Feb 13, 2022

I think a command to move/copy wasm files to the package directory would be enough

I guess this won't be needed even? I have to double confirm though. Since plugin is a single pkg doesn't care about different platform / binaries, as long as package.json points correct entrypoint to binary path resolver should able to find it. This could be a bit cumbersome if someone wants absolute path to the wasm binary for the installed package, but also it's pretty much predictable so not a huge concern.

I need to work on resolver part first to confirm this though. Will update both part.

@kdy1
Copy link
Member

kdy1 commented Feb 13, 2022

I was talking about swc-project/plugins#19, as I expect many companies to use a monorepo for their plugins.
wasm files are stored in target/wasm32-wasi/release/<crate_name>.wasm, and it's hard to get cargo crate name from a shell script.

@kwonoj
Copy link
Member Author

kwonoj commented Feb 13, 2022

I may miss some points, so are you referring the case 1. monorepo 2. which they do not create separate npm packages but manually resolve paths to each output for their own?

1 should not be relavant regardless of output path, I can only think someone need manual resolve for the case of 2 and if they do not rely on any metadata from pkg.json or cargo.toml.

@kdy1
Copy link
Member

kdy1 commented Feb 13, 2022

I mean a monorepo with separate npm packages. Problem is that to publish an npm package, we should move the wasm file to a directory which has package.json, and for monorepo this directory is different between packages.
So we need some clever trick or e.g.

mv target/wasm32-wasi/release/swc_plugin_jest.wasm packages/jest
mv target/wasm32-wasi/release/swc_plugin_styled_components.wasm packages/styled_components

and once per package, because the cargo package name may not match the directory name.

@kwonoj
Copy link
Member Author

kwonoj commented Feb 13, 2022

Problem is that to publish an npm package, we should move the wasm file to a directory which has package.json

Yes, I think this is the part I'm probably not getting at fully. I was under assumption pkg structure looks like this

- big-monorepo
   - some_js
   - some_backend
   - swc_plugin
      - a
         package.json // main: target/.../a.wasm
         target/.../a.wasm

(this is actually similar to some of repo I worked on)

and it seems it's not the layout you're expecting?

I'd like to collect some firm / real world usecases of how does layout works across actual users before we tweak something. There are various possible project structures afaik and I don't think we can predict all. We can start from obvious, simple approach first then add supporting some other usecases.

@RiESAEX
Copy link
Contributor

RiESAEX commented Feb 13, 2022

I think it can be like

{
    "main": "my-swc-plugin.wasm",
    "scripts": {
      "pre-release": "cargo build --release && cp target/wasm32-wasi/release/my-swc-plugin.wasm .",
      "publish":"npm publish"
    },
    "files":[
        "my-swc-plugin.wasm",
    ],
// ...
  }

just put package.json next to cargo.toml

@kdy1
Copy link
Member

kdy1 commented Feb 13, 2022

Yeah, I'll do it for now.
Thank you!

I forgot that // main: target/.../a.wasm is possible, and thought that it's not usable if wasm file is in some different directory.

@kwonoj
Copy link
Member Author

kwonoj commented Feb 13, 2022

I'm trying to understand: why we have to copy?

I was thinking

package.json
{
   // we know this path when `plugin new` command runs as we know what target we'll build
   main: 'target/.../a.wasm'
}

This removes needs of redundant post / pre script hooks.

@kdy1
Copy link
Member

kdy1 commented Feb 13, 2022

Because the cargo target directory can be outside of the npm package directory.

@kwonoj
Copy link
Member Author

kwonoj commented Feb 13, 2022

Because the cargo target directory can be outside of the npm package directory.

That means this copy

cp target/wasm32-wasi/release/my-swc-plugin.wasm .

also won't work transparently, isn't it? at the moment of project creation, we don't know if /target will be next to the package.json or not. It's the reason I suggested going with straightforward approach first and see if there are other possible cases.

@kdy1
Copy link
Member

kdy1 commented Feb 13, 2022

Yeah I also think we can patch after collecting feedback.
I'll use cp for swc-project/plugins for now

@RiESAEX
Copy link
Contributor

RiESAEX commented Feb 13, 2022

main: 'target/.../a.wasm'

I'm not familier with npm publish,
I'm worried about this will create a release like

- target
  - wasm32-wasi
    - release
      - my-swc-plugin.wasm

@kdy1
Copy link
Member

kdy1 commented Feb 13, 2022

@RiESAEX I think it's fine, as the main field of package.json will store the correct path.
(I don't think it's ideal, but it's fine at the moment)

@kwonoj
Copy link
Member Author

kwonoj commented Feb 13, 2022

I'm worried about this will create a release like

Yes it will, but that's quite common for other packages. Only difference is nested level. Peek rxjs for an example - https://unpkg.com/browse/[email protected]/dist/cjs/ it's cjs dist output is dist/cjs/index.

@swc-bot
Copy link
Collaborator

swc-bot commented Oct 18, 2022

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@swc-project swc-project locked as resolved and limited conversation to collaborators Oct 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging a pull request may close this issue.

4 participants