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 pkg module reference to original entry #568

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

izelnakri
Copy link

@izelnakri izelnakri commented Jun 23, 2021

@stefanpenner I had a very weird silent build bug when I tried to run router_js with my emberx experiment: https://github.com/izelnakri/emberx/tree/rsvp-fail . The tests were failing with both esbuild and tsc outputs, after some in-depth investigation I found that this reference is causing the problem. I fixed it in this commit, and tests started passing(you can ignore the Dockerfile change): izelnakri/emberx@5312bf2

What is happening?

This library has not been published for 2 years, mainly I suppose there was no need for it. It has 0 dependencies, however the package itself tries to do some runtime imports and its runtime changes based on that: https://github.com/tildeio/rsvp.js/blob/master/lib/rsvp/asap.js#L90

When I change the "module" entry scheduleFlush get assigned to different functions between dist/rsvp.es.js(original) and lib/rsvp.js(current fix). Original entry results in runtime error on my specific tests in browser that are built on top of router_js.

This makes router_js which depends on rsvp not runnable correctly on node.js and browser environments until we apply the change in this PR.

Steps to reproduce

Although this is a temporary solution, I think we should merge it because the library itself is written in es modules, so module reference shouldn't need a build step, including for ember-cli environments. This removes the extra layer of indirection between the library code and its runtime as we simplfy the library build process for modern bundlers. Transpiling the code with both esbuild and tsc both gives the same runtime error.

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.

1 participant