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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: preact alias, rollup configs + compat-lite #15

Closed
wants to merge 1 commit into from

Conversation

zouhir
Copy link
Collaborator

@zouhir zouhir commented Jul 26, 2017

馃毇 Not ready for merge.

I've only sent a PR from my private fork for reviews and opinions.

Having a little issue with #4 when I rollup using the configs below, only reason I've posted here is I have not fiddled much recently with React tools and specs in depth. I'll be investigating that..

  • remove dom-scroll-into-view
  • update proptypes to compat number, obj, any
  • add build default to build both preact & react one
  • pass environment variable from Rollup to save a bunch of no-prod React stuff.

Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking really great!

@@ -0,0 +1,13 @@
export * from 'preact'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well this is pretty rad

@@ -0,0 +1,11 @@
function proptype() {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coolio

dest: pkg.browser,
format: 'umd',
moduleName: pkg.name,
external: ['dom-scroll-into-view', 'react'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want people to have to install dom-scroll-into-view. I realize it's not exactly small, but I'd rather try to rewrite a smaller version (and contribute back if they'll accept it), or bundle it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool! I do agree - if we can not include it would be better! but I thought it's an external dependency. will take it away and iterate tomorrow.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a small rewrite for scroll-into-view that I can PR :) I ran into the same issue with react-aria.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super!

@@ -8,6 +8,8 @@
"npm": "> 3"
},
"scripts": {
"build": "rollup -c --environment LIBRARY:react -o dist/index.js",
"build:preact": "rollup -c --environment LIBRARY:preact -o dist/preact/index.js",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you put these in the package-scripts.js file as part of the build script, and add them to the validate script?

If you're unfamiliar with nps that's fine, I can merge this and fix that up myself later :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh crap! I oversaw that, yeah that's easy I can do it..

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be best if we could make the build.default be concurrent.nps('build.react', 'build.preact') or something 馃憤

@kentcdodds
Copy link
Member

Alrighty! I've created a build and added preact support!! 馃帀

Thank you @zouhir! I used this as an example for a bunch of stuff. Much appreciated 馃槃

NitsanBaleli pushed a commit to NitsanBaleli/downshift that referenced this pull request Mar 15, 2021
* Fix ESC key in examples

Pressing the ESC key clears the input and calls onChange with null. Some examples did not handle this case and crash as a result. downshift-js#719

Update debounce-fn to fix gmail and axios examples

* Set debounce-fn to ^3.0.1
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

3 participants