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

Include full file path in CJS requires. #3314

Merged
merged 1 commit into from Apr 23, 2018

Conversation

Projects
None yet
3 participants
@chexxor
Copy link
Contributor

commented Apr 21, 2018

Fixes #2621

Changes the JS output from require("../SomeModule") to require("../SomeModule/index.js"), and same for the implicit FFI imports.

stack test is proving difficult. First time I ran it, it had trouble installing bower dependencies, giving ECONFLICT Unable to find suitable version for purescript-either. After manually installing them, I got errors about Monoid being a duplicate module, which occurred because it's been moved to Prelude in the latest version (so we may need to adjust the version range in the bower file). After manually deleting Monoid stuff from the PS Prelude it was using, I get test failures caused by stuff missing from Prim -- Module Prim.RowList was not found., and same for Prim.Ordering and Prim.Symbol.

@garyb

garyb approved these changes Apr 21, 2018

Copy link
Member

left a comment

LGTM!

@chexxor

This comment has been minimized.

Copy link
Contributor Author

commented Apr 21, 2018

Ah, I bet some of these problems are fixed in #3312, as the deps I've been tinkering with appear to have been bumped and fixed in that PR. Also, it looks like stuff missing from Prim could be fixed by that PR, as it's moving them from lib to compiler (if I read it right).

The other question I have is about tests -- do we need to have some tests written to assert this? If you ask me, I don't think it's necessary.

@garyb

This comment has been minimized.

Copy link
Member

commented Apr 21, 2018

Yeah, everything will be failing until we finish with #3312, that's the problem with #3309 too.

The tests will be subject to the changes you've made here already, so no, there's no further specific tests I can think of that will be necessary :)

@chexxor

This comment has been minimized.

Copy link
Contributor Author

commented Apr 21, 2018

Here's why I want this patch, BTW. https://www.screencast.com/t/KgVyC3ksG or https://1drv.ms/v/s!AnIo6jtaL-CjkG-zDxXZxVHIc-8s

No need to add a bundle or transpile step PureScript code during development -- just refer directly to the single PS module and the dependencies will be dynamically loaded. (Yeah, SystemJS does transpile, but it seems to be much easier to get started with, I think.)

The bundling step is often a hurdle for Haskell people coming to PureScript. Yes, tooling like pulp --to x.js can help with that, but it complicates the project set-up. Also this patch to the compiler doesn't break anything and is a benefit-only change -- it adds the ability to very easily use SystemJS in-browser loader instead of a server-side bundler.

@LiamGoodacre LiamGoodacre merged commit 8ae2a06 into purescript:master Apr 23, 2018

0 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
@LiamGoodacre

This comment has been minimized.

Copy link
Member

commented Apr 23, 2018

Thanks!

@chexxor chexxor deleted the chexxor:full-path-requires branch Apr 23, 2018

@chexxor chexxor referenced this pull request May 2, 2018

Merged

Add myself to Contributors #3346

@hdgarrood hdgarrood referenced this pull request Jan 13, 2019

Merged

Update 0.12 #362

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.