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

Compiler.transpile creates code that can't run in the browser #2776

Closed
theo-staizen opened this issue Dec 21, 2020 · 6 comments
Closed

Compiler.transpile creates code that can't run in the browser #2776

theo-staizen opened this issue Dec 21, 2020 · 6 comments
Labels
Bug: Validated This PR or Issue is verified to be a bug within Stencil

Comments

@theo-staizen
Copy link

theo-staizen commented Dec 21, 2020

Stencil version:

I'm submitting a:

[x] bug report

I am trying to transpile stencil components for demo purposes, following the docs found here: https://stenciljs.com/docs/compiler-api

Current behavior:
Getting an exception because the transpileResult.code is trying to import from @stencil/core/internal/client which the browser cannot resolve.

import { defineCustomElement as __stencil_defineCustomElement } from "@stencil/core/internal/client";
...

Expected behavior:
Perhaps the transpile function should take a baseURL or an importMap ?

Related code:
See (FIXED) codepen example here: https://codepen.io/dogoku/pen/zYKBbOy?editors=1010

Other information:
In order to get the codepen above working, I had to do the following:

  • Use codepen's external JS feature to to load @stencil/core/compiler/stencil.min.js from jsdelivr cdn (skypack does not load this correctly at the moment)
  • Replace @stencil/core/internal/client with https://cdn.skypack.dev/@stencil/core/internal/client in the transpileResult.code to make the import load correctly
  • Use skypack.dev to import h function from @stencil/core/, because skypack transforms npm packages so that imports that rely on node resolver will work correctly in the browser

Ideally we should have a core package that can work in the browser, given that compiler docs claim that u can run in browser or even better, expose a JSX render function, similar to React.render

@ionitron-bot ionitron-bot bot added the triage label Dec 21, 2020
@alicewriteswrongs
Copy link
Contributor

@theo-staizen thanks for filing this issue and sorry it took so long for someone from the Stencil team to get back to you!

I had a bit of trouble getting that codepen to run. I suspect this issue is probably still present in the Stencil codebase, but it would be good to confirm with an up-to-date reproduction.

Any chance you could provide one? I know it's been a while since this was opened, but it helps us enormously to have a repro case to work with.

@alicewriteswrongs alicewriteswrongs added the ionitron: needs reproduction This PR or Issue does not have a reproduction case URL label Feb 7, 2023
@ionitron-bot
Copy link

ionitron-bot bot commented Feb 7, 2023

Thanks for the issue! This issue has been labeled as needs reproduction. This label is added to issues that need a code reproduction.

Please reproduce this issue in an Stencil starter component library and provide a way for us to access it (GitHub repo, StackBlitz, etc). Without a reliable code reproduction, it is unlikely we will be able to resolve the issue, leading to it being closed.

If you have already provided a code snippet and are seeing this message, it is likely that the code snippet was not enough for our team to reproduce the issue.

For a guide on how to create a good reproduction, see our Contributing Guide.

@ionitron-bot ionitron-bot bot removed the triage label Feb 7, 2023
@theo-staizen
Copy link
Author

theo-staizen commented Feb 8, 2023

@alicewriteswrongs thanks for the taking the time to look at this, but honestly I'd rather you look at #2731 instead (specifically read my last comment) , which is actually affecting production. The compiler is more of a dev tool / nice to have and since I have a workaround, I don't really pay attention to it.

In any case, to answer your original question, here is a gist i created to make things cleaner and a codepen that uses it.
The workaround logic is in the compileStencil function.

To summarise, there are 2 problems that make using the compiler annoying to work with:

  1. @stencil/core/compiler/stencil.min.js is not a module, but rather just puts stencil on the global scope, which makes it harder to import when needed (need to dynamically create script tags or use a worker)
  2. The compiler outputs code that has node import paths, instead of valid URLs, which need to be either manually replaced in the compiled code or used together with import paths, so that the browser understands them

@alicewriteswrongs
Copy link
Contributor

@theo-staizen definitely agree that the web worker issue (#2731) is higher-priority than this one, we're just trying to do a little bit every day to put some attention to older issues in the issue tracker. So thank you for updating the repro, I can confirm reading through that this isn't working as it should. Thanks for helping us out in this effort by updating the reproduction case!

To be honest, running Stencil in the browser is, right now, a bit sketchy — while it is supported, the current Stencil team does not have a lot of expertise is the ins and outs of how it works, and the approaches necessary throughout the codebase are fairly complicated. But nonetheless, it's still good to know concretely some of the issues with it!

Thanks again for updating the reproduction case - I'm going to mark this so it gets ingested into our internal backlog so we can evaluate and prioritize it.

@alicewriteswrongs alicewriteswrongs added Bug: Validated This PR or Issue is verified to be a bug within Stencil and removed ionitron: needs reproduction This PR or Issue does not have a reproduction case URL labels Feb 8, 2023
@theo-staizen
Copy link
Author

theo-staizen commented Feb 9, 2023

@alicewriteswrongs something somewhat related i figured i should point out in case the team wasn't aware

if you look at https://github.com/ionic-team/stencil/blob/main/test/browser-compile/, this is an internal package used for testing the compiler (which i have also ripped and deployed into this sandbox, since its much more comprehensive than my original codepen)

The interesting thing to note is the load-deps.ts file, which is used to handle importing the dependencies, similar to what I am doing in my gist. Also to note is that they are using rollup in the browser (!) along with an inlined "browser" plugin to bundle the deps.

In contrast, I am using skypack, which transforms the files to support importing them natively via the browser.
Regardless, this just goes to show the extend at which you need to go to get this compiler working in a browser 😅

@alicewriteswrongs
Copy link
Contributor

Hey @theo-staizen, I'm going to go ahead and close this as built-in support for compiling Stencil components in the browser was removed as part of our recent 4.0.0 release. That change followed an RFC on the decision and will enable us to move forward with a lot of changes and improvements to Stencil in the near future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug: Validated This PR or Issue is verified to be a bug within Stencil
Projects
None yet
Development

No branches or pull requests

2 participants