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

WIP: Flow example #49

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

WIP: Flow example #49

wants to merge 12 commits into from

Conversation

teh
Copy link
Member

@teh teh commented May 24, 2017

Don't merge, just for illustration.

@amenk
Copy link
Collaborator

amenk commented Jul 29, 2017

the current state of the branch is not working .. I have problems with the imports. How can I resolve that? My idea was to try to work on that a bit ...

selection_336

@amenk
Copy link
Collaborator

amenk commented Jul 29, 2017

I did the normal build, as documented in the readme.

@amenk
Copy link
Collaborator

amenk commented Jul 29, 2017

I did not see the stuff in the branch-readme .. will try

@amenk
Copy link
Collaborator

amenk commented Jul 29, 2017

But something seems to be wrong with the lockfile:

yarn install v0.27.5
[1/4] Resolving packages...
error An unexpected error occurred: "Cannot convert object to primitive value".
info If you think this is a bug, please open a bug report with the information provided in "passopolis/extensions/yarn-error.log".
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.

when I delete the yarn.lock, yarning works

@amenk
Copy link
Collaborator

amenk commented Jul 29, 2017

So I executed

 node_modules/.bin/babel login/ --watch --out-dir build

and then

  nix-build nix/build.nix -A chrome-extension && mkdir -p /tmp/passopolis-chrome && rsync -a result/ /tmp/passopolis-chrome/

Extension is not working...

@teh
Copy link
Member Author

teh commented Jul 29, 2017

Hey - yea this definitely doesn't work in its current state. Migration is a non-trivial task because everything needs to be modularized and the code base is not small. I can't remember the details but as you saw imports need to be resolved first by a packer - I hadn't gotten around to that bit yet.

I.e. this branch is not for the faint hearted

@amenk
Copy link
Collaborator

amenk commented Jul 29, 2017

Which kind of packer could that be? Does also flow provide such a thing?

@teh
Copy link
Member Author

teh commented Jul 29, 2017

I don't know - I hadn't gotten around to that part yet. IIRC even if we can resolve the imports there are still some issues with incomplete flow annotations themselves though I can't quite recall the details.

@amenk
Copy link
Collaborator

amenk commented Jul 29, 2017 via email

@Keats
Copy link
Member

Keats commented Jul 30, 2017

Those are ES6 imports that can be resolved by webpack, rollup or browserify.

@amenk
Copy link
Collaborator

amenk commented Jul 30, 2017

Any preferences which way we should go?

Shall I try webpack? Any objections?

There is already the plugin "transform-es2015-modules-amd" in .babelrc` but this probably does not pack anything together?

@amenk
Copy link
Collaborator

amenk commented Jul 30, 2017

The problem seems to be a different one: When nix-building the //@flow files still arrive in the extension, where should be the processed javascript files.

@amenk
Copy link
Collaborator

amenk commented Jul 30, 2017

  • in chrome-extension.nix change line 16 to cd build/ (from cd login/) so we are using the actually processed source
  • cp -r build login
  • call babel
  • call nix

Works a bit more, not it does not find the define() statements (Uncaught ReferenceError: define is not defined) - I think for getting those working we need requireJS ? Or would webpack solve this in another way?

@amenk
Copy link
Collaborator

amenk commented Jul 30, 2017

I managed to add require.cs to common (then copy it over to build, so it is NOT processed by babel) ... no I get different errors ... but it looks like a good way to go.

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.

3 participants