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

fix: add source entry and includes src files #501

Closed
wants to merge 1 commit into from

Conversation

aminya
Copy link
Contributor

@aminya aminya commented Jun 22, 2021

See solidjs/solid-router#17 (comment)

This mostly works, but there is a dom-expressions dependency that is not declared anywhere in package.json of solid-js, and makes issues for the direct source build. I don't know how you publish the packages, so this might be an issue with the way I use npm pack.

× Build failed.
@parcel/core: Failed to resolve 'dom-expressions/src/client' from './node_modules/.pnpm/local+C+solid+solid+packages+solid+solid-js-1.0.0-rc.7.tgz/node_modules/solid-js/web/src/client.js'
C:\test\node_modules\.pnpm\local+C+solid+solid+packages+solid+solid-js-1.0.0-rc.7.tgz\node_modules\solid-js\web\src\client.js:1:15
> 1 | export * from "dom-expressions/src/client";
>   |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 ERROR  Command failed with exit code 1.

@coveralls
Copy link

coveralls commented Jun 22, 2021

Pull Request Test Coverage Report for Build 959893843

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 90.182%

Totals Coverage Status
Change from base Build 959745216: 0.0%
Covered Lines: 1054
Relevant Lines: 1121

💛 - Coveralls

@ryansolid
Copy link
Member

ryansolid commented Jun 22, 2021

It's a dev dependency because I compile it into the final output using tooling to rewrite its imports. It is top level but I imagine moving it in won't help here. Weird system but I basically need a build system to generate Solid's source and maintain tree-shakeability.

@trusktr
Copy link
Contributor

trusktr commented Dec 7, 2021

I also trip on the non-standard project layout sometimes. Instead of having a fake import statement importing a non-existent rxcore package, what if we default it to solid-js/web instead, and replace that depending on moduleName? (etc)

I'm not sure what issue there was, but I'm guessing it is possible to make this work and keep tree shaking.

@lxsmnsyc
Copy link
Member

lxsmnsyc commented Dec 7, 2021

I'm not sure what issue there was, but I'm guessing it is possible to make this work and keep tree shaking.

IIRC the issue is more of when packing the package and not during building.

@ryansolid
Copy link
Member

This issue is stale and not going anywhere. Parcel is the only bundler that respects source but it also doesn't transform node_modules. The build process pretty much makes this not a thing. We might revise DOM Expressions at some point but at that time we will have to come up with a plan.

@ryansolid ryansolid closed this Jan 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants